helix icon indicating copy to clipboard operation
helix copied to clipboard

Published 20 hours ago • verified publisher icon helix-editor

Added missing static commands to documentation

Open adamperkowski opened this issue 1 year ago • 13 comments

Added missing commands (those without keymaps) to documentation.

Documented in Issue #10970.

adamperkowski avatar Jul 12 '24 20:07 adamperkowski

Working on it.

adamperkowski avatar Jul 12 '24 21:07 adamperkowski

Did everything as it's supposed to be. Sorry for the inconvenience.

adamperkowski avatar Jul 12 '24 23:07 adamperkowski

You think it would be better to create a separate doc page 2.4 for static commands? (i added a new section here)

adamperkowski avatar Jul 13 '24 13:07 adamperkowski

Yeah the page will be quite long with static and typable commands together. Let's use separate pages

the-mikedavis avatar Jul 13 '24 13:07 the-mikedavis

Done.

adamperkowski avatar Jul 13 '24 13:07 adamperkowski

Hi @the-mikedavis I just wanted to check in on the pull request. If you have any more feedback, please let me know. Whenever you have a moment, could you please take a look and consider merging? I would really appreciate it.

adamperkowski avatar Jul 15 '24 19:07 adamperkowski

Code-wise this looks good, just a note about the layout

the-mikedavis avatar Jul 15 '24 22:07 the-mikedavis

Done.

adamperkowski avatar Jul 15 '24 22:07 adamperkowski

(The CI failure is from master, not related to these changes)

the-mikedavis avatar Jul 15 '24 23:07 the-mikedavis

Hey, just checking on again. Will this PR be merged?

adamperkowski avatar Jul 20 '24 01:07 adamperkowski

Implementation looks good. I do wonder if we should emulate the command picker and also show default bindings here. While we already have a handwritten keymap docs it can be useful to see wether default bindings exist (and what they are) right away after finding a command here.One challenge with that is that bindings would be mode dependent I guess. I guess for now one putting the mode a command is mapped for in parenthesis could work. For example c-z (normal,select,insert),j (normal)

This would also cover some things missing from the keymaps docs (like c-z for suspend). I think often users can be familiar with a keybinding but may nit understand/know the bame/description (c-z is widely know, but the suspend command noch so much).

Thoughts @the-mikedavis

pascalkuthe avatar Jul 24 '24 00:07 pascalkuthe

@pascalkuthe I could open another PR for that

adamperkowski avatar Jul 24 '24 00:07 adamperkowski

Hmm yeah that's a good idea. See the reverse_map function in keymap.rs and the command_palette implementation in commands.rs for examples of how to that display is working in the command palette (<space>?). It would be better to introduce that change in one go (i.e. this PR): any new PRs that add commands between merging this PR and a follow-up would cause conflicts and CI failures.

the-mikedavis avatar Aug 08 '24 20:08 the-mikedavis

@pascalkuthe @the-mikedavis Done.

adamperkowski avatar Sep 11 '24 20:09 adamperkowski

I noticed an issue with special characters like ` or <> in keybinds. It's messing up the markdown. Is this a thing with generating webpages too? If that's the case, I really think not adding the keybinds is a better idea.

I could make docgen put a \ before them.

adamperkowski avatar Sep 11 '24 21:09 adamperkowski

Added your suggestions, @the-mikedavis. I don't understand one thing though. Why are there duplicates? This one for example.

image

adamperkowski avatar Sep 14 '24 21:09 adamperkowski

Can't reopen the PR for some reason. I'll open a new one

adamperkowski avatar Sep 14 '24 22:09 adamperkowski