Skip to content
This repository was archived by the owner on Aug 3, 2024. It is now read-only.

Fix #783 Don't show button if --quickjump not present #1108

Merged
merged 1 commit into from
Jun 13, 2022

Conversation

mpilgrem
Copy link
Contributor

In the TypeScript init.ts, makes quickJump.init() (which creates the button) conditional on the presence of meta tag hasQuickJump with content "true".

In the Haskell, threads the withQuickJump flag result through to headHtml to create the meta tag, accordingly.

@mpilgrem
Copy link
Contributor Author

The CI build fails because of the existing commit e6ca100. Without that commit, it builds under Windows 10 using stack.

@mpilgrem
Copy link
Contributor Author

I have thought of a different approach to this, which I think is better, and will provide a revised pull request. The creation of the dummy li element is moved from the TypeScript to the Haskell and identified, and its existence (or not) replaces the use of the meta tag.

Copy link
Member

@Kleidukos Kleidukos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpilgrem Hi, and thank you very much for this PR! I left you a couple of comments and it should be good.
That being said, would you be able to include a screenshot of the updated interface when --quickjump isn't passed as an argument?

@mpilgrem
Copy link
Contributor Author

Below is a screenshot of the updated interface when --quickjump is not passed as a command line option:

image

@mpilgrem
Copy link
Contributor Author

@Kleidukos, I have updated the PR to reflect your two comments and posted a screenshot.

@Kleidukos
Copy link
Member

@mpilgrem Brilliant! We are currently in a merge phase to backport PRs to the ghc-9.0 branch, so while I won't be merging your PR as it is now (since it's targeting the now-defunct ghc-8.8 branch), it will be ported to the next main branch.

Thanks again ✨

@Kleidukos
Copy link
Member

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Jun 6, 2022

rebase

✅ Branch has been successfully rebased

@Kleidukos Kleidukos changed the base branch from ghc-8.8 to ghc-9.2 June 6, 2022 10:08
@Kleidukos
Copy link
Member

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Jun 6, 2022

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/6)
dropping 89ec5657326eefbcdcead6e8a5f28f72fd21cf94 Fix multiple typos and inconsistencies in doc/markup.rst -- patch contents already upstream
Rebasing (2/6)
dropping 64cdf0cf325b313ff527eb8871faacada3c92fc0 Fix #1113 If no Signatures, no section of index.html -- patch contents already upstream
Rebasing (3/6)
error: could not apply 985dad3c... Change the formatting of missing link destinations
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 985dad3c... Change the formatting of missing link destinations
Auto-merging haddock-api/src/Haddock/Interface/Rename.hs
CONFLICT (content): Merge conflict in haddock-api/src/Haddock/Interface/Rename.hs

err-code: 6287F

@mpilgrem
Copy link
Contributor Author

@Kleidukos I saw your request for a rebase and the bot's failed attempt. I have rebased manually against the ghc-9.2 branch and tested the rebased code locally by: (1) building haddock.exe locally with stack and nightly-2022-06-12 (GHC 9.2.3); and (2) creating Haddock documentation for a simple test project (stack new test) with haddock -o docs -h app/Main.hs src/Lib.hs and haddock -o docs -h --quickjump app/Main.hs src/Lib.hs and reviewing the results using the Live Server extension for Visual Studio Code.

I experienced some initial difficulties with rebuilding the minified JS files in haddock-api\resources\html. I found I had to npm install [email protected] (previously it was local version 3.9.1) to get the gulp command to work without errors.

@Kleidukos
Copy link
Member

Kleidukos commented Jun 13, 2022

One more reason to reshape our frontend stack

@Kleidukos Kleidukos added the merge me Tell Mergify bot to merge the PR label Jun 13, 2022
@mergify mergify bot merged commit 118dd4e into haskell:ghc-9.2 Jun 13, 2022
@mpilgrem mpilgrem deleted the fix783 branch June 13, 2022 12:42
@mpickering
Copy link
Collaborator

This PR breaks the quickjump feature on hackage.haskell.org.

See https://gitlab.haskell.org/ghc/ghc/-/issues/21984

We will need to revert it for the 9.4.2 release.

@mpilgrem
Copy link
Contributor Author

@mpickering, sorry! I only ever tested locally. Is there a recommended way to test that pull requests for haddock will not break the Haddock functionality on Hackage?

@mpickering
Copy link
Collaborator

I tested the changes by using a local hackage server (which is quite easy to setup, it's documented in the README).

I also noticed that quick jump was not supported for candidates, so I added that (haskell/hackage-server#1122) so perhaps another way to test in future is upload a candidate for one of your packages to check everything is in order.

hubot pushed a commit to ghc/ghc that referenced this pull request May 17, 2024
Fix haskell/haddock#783 Don't show button if --quickjump not present
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
merge me Tell Mergify bot to merge the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants