Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autodoc: make imports into links #20007

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

autodoc: make imports into links #20007

wants to merge 5 commits into from

Conversation

tjog
Copy link
Contributor

@tjog tjog commented May 19, 2024

This PR contains three changes:

Keybindings on non US keyboards

The keybindings previously switched on the code (MDN) rather than the key (MDN). Many keyboards are QWERTY based, but I noticed this when pressing ?. Either UI should change or this key handling should be used instead.

Tarballs with backslashes

Tarball unpacking replaces backslashes with forward slashes. Discovered this during local development on Windows where the found files logged std/Thread\Pool.zig. Perhaps a change to the std.tar implementation to reject setPath with backslashes is in order. I am not aware if it should be forbidden by the tar implementation, or the caller to understand backslashes will not register as separate directories. Either way, this unpacking change will handle more tarballs, regardless of source.

Make imports into links

Before:
image

After (while hovering the second import statement):
image

Currently, the import token is the one with link and highlighting (when hovered). The alternative is the import string being linkified. I could try to make that change, but this was the most straightforward version. Field access can also terminate in an import statement now, leading to more identifiers being linked, e.g. the @import("array_list").XyzVersion pattern.

This ticks one of the TODOs in #19249

@tjog
Copy link
Contributor Author

tjog commented May 19, 2024

Found #19276 which changed to code rather than key to support non-latin keyboards. Read a bit more and the extensive solution seems to be doing something like

https://www.kravchyk.com/keyboard-shortcuts-on-non-latin-alphabet-layouts/

which basically uses key by default, but if a non-latin character is detected it falls back to code and uses some custom logic to extract a latin character from that code. (source code)

Let me know if the commit should be dropped.

@Vexu
Copy link
Member

Vexu commented May 20, 2024

Currently, the import token is the one with link and highlighting (when hovered). The alternative is the import string being linkified.

Can you try linkifying both the builtin token and the string argument?

Or just linkify the string to match how ZLS does it.

@tjog
Copy link
Contributor Author

tjog commented May 20, 2024

Or just linkify the string to match how ZLS does it.

Done, link (and hover) only applied to the import string now:
image

Was easier this time around now that I understood where data is located in the DOD Tokenizer and Ast structures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants