Add support for subcommands in __fish_man_page #3678

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@radomirbosak
Contributor

radomirbosak commented Dec 26, 2016

This PR adds a feature that after typing git add and pressing
alt+h, the manpage for git-add instead of git would be displayed.

The new logic takes the first argument which doesn't start with a dash
and tries to display manpage for command-argument; it falls back to
man command it the first try doesn't succeed.

Fixes #3618.

share/functions/__fish_man_page.fish
+
+ # If there are at least two tokens not starting with "-", the second one might be a subcommand.
+ # Try "man first-second" and fall back to "man first" if that doesn't work out.
+ set -l maincmd (basename $args[1])

This comment has been minimized.

@faho

faho Dec 26, 2016

Member

This will now print an error when the commandline is empty.

There's a few ways to stop that, I think I prefer a set -q args[1]; or return above.

@faho

faho Dec 26, 2016

Member

This will now print an error when the commandline is empty.

There's a few ways to stop that, I think I prefer a set -q args[1]; or return above.

This comment has been minimized.

@radomirbosak

radomirbosak Dec 26, 2016

Contributor

You are right.. I didn't test the empty commandline scenario after adding basename.

I'm adding the check and additionally printing \a so that the behavior in case of empty commandline is consistent with other cases (non-existent manpage).

@radomirbosak

radomirbosak Dec 26, 2016

Contributor

You are right.. I didn't test the empty commandline scenario after adding basename.

I'm adding the check and additionally printing \a so that the behavior in case of empty commandline is consistent with other cases (non-existent manpage).

share/functions/__fish_man_page.fish
- man (basename (commandline -po; echo)[1]) ^/dev/null
- or printf \a
+ # Get all commandline tokens not starting with "-"
+ set -l args (string join \n -- (commandline -po) | grep -v '^-')

This comment has been minimized.

@faho

faho Dec 26, 2016

Member

What is the string join supposed to do here?

Also, the grep can be replaced with string match.

So: commandline -po | string match -rv '^-' should work.

@faho

faho Dec 26, 2016

Member

What is the string join supposed to do here?

Also, the grep can be replaced with string match.

So: commandline -po | string match -rv '^-' should work.

This comment has been minimized.

@radomirbosak

radomirbosak Dec 26, 2016

Contributor

The string join \n was there because grep expects the tokens newline delimited, while commandline -po produces them space-delimited (maybe more precisely - it's that array-separator character).

In any case - your suggestion is simpler - so I'm changing the code to that.

@radomirbosak

radomirbosak Dec 26, 2016

Contributor

The string join \n was there because grep expects the tokens newline delimited, while commandline -po produces them space-delimited (maybe more precisely - it's that array-separator character).

In any case - your suggestion is simpler - so I'm changing the code to that.

This comment has been minimized.

@faho

faho Dec 26, 2016

Member

while commandline -po produces them space-delimited

It doesn't - it delimits with newline. Presumably whatever you tested this with changed the separator. I'd be interested to know what that was, since we don't want to be leaking \x1e (that "array-separator").

@faho

faho Dec 26, 2016

Member

while commandline -po produces them space-delimited

It doesn't - it delimits with newline. Presumably whatever you tested this with changed the separator. I'd be interested to know what that was, since we don't want to be leaking \x1e (that "array-separator").

This comment has been minimized.

@radomirbosak

radomirbosak Dec 26, 2016

Contributor

Oh, my mistake.. I tested it with echo (commandline -po) | grep ... and didn't realize it was echo which was creating the spaces.

@radomirbosak

radomirbosak Dec 26, 2016

Contributor

Oh, my mistake.. I tested it with echo (commandline -po) | grep ... and didn't realize it was echo which was creating the spaces.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Dec 26, 2016

Member

Now that I've addressed the code, on to the general idea.

I'm actually suprised how well this rather simple approach works - so far I haven't found a case where it fails (well, it does with ip addr, but that's because the actual subcommand is "address" and "addr" is just a short form - not something that needs to be supported).

I'd be grateful if others could test this so that we don't have any embarrassing failures in obvious situations that I'm too dense to see.

Member

faho commented Dec 26, 2016

Now that I've addressed the code, on to the general idea.

I'm actually suprised how well this rather simple approach works - so far I haven't found a case where it fails (well, it does with ip addr, but that's because the actual subcommand is "address" and "addr" is just a short form - not something that needs to be supported).

I'd be grateful if others could test this so that we don't have any embarrassing failures in obvious situations that I'm too dense to see.

@faho faho added the enhancement label Dec 26, 2016

@faho faho added this to the fish 2.5.0 milestone Dec 26, 2016

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Dec 26, 2016

Contributor

Looks okay to me. A problem with the original and new version of this function is that when I exit the man pager the command line isn't refreshed. Adding commandline -f repaint to the bottom of the function fixes it.

Contributor

krader1961 commented Dec 26, 2016

Looks okay to me. A problem with the original and new version of this function is that when I exit the man pager the command line isn't refreshed. Adding commandline -f repaint to the bottom of the function fixes it.

@radomirbosak

This comment has been minimized.

Show comment
Hide comment
@radomirbosak

radomirbosak Dec 26, 2016

Contributor

@krader1961 Interesting, I don't have the issue you are describing. After exiting the man pager, the commandline is (visually) in the same state as it was before entering the man pager.

Which terminal are you using? I'm using gnome-terminal.

Contributor

radomirbosak commented Dec 26, 2016

@krader1961 Interesting, I don't have the issue you are describing. After exiting the man pager, the commandline is (visually) in the same state as it was before entering the man pager.

Which terminal are you using? I'm using gnome-terminal.

Add support for subcommands in __fish_man_page
This commit adds a feature that after typing "git add" and pressing
"alt+h", the manpage for "git-add" instead of "git" would be displayed.

The new logic takes the first argument which doesn't start with a dash
and tries to display manpage for "command-argument"; it falls back to
"man command" it the first try doesn't succeed.

Fixes #3618.
@radomirbosak

This comment has been minimized.

Show comment
Hide comment
@radomirbosak

radomirbosak Dec 26, 2016

Contributor

Added commandline -f repaint to the bottom of the function.

Contributor

radomirbosak commented Dec 26, 2016

Added commandline -f repaint to the bottom of the function.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Dec 27, 2016

Contributor

Added commandline -f repaint to the bottom of the function.

Thanks. The reason you don't need it, and probably most people, is that you're using less or another pager that switches to the alternate screen by default. I don't like that behavior so I put --no-init in my $LESS env var. Which means less scribbles on the screen containing the prompt for me. I've never bothered to fix this because I've only used [alt]H a couple of times when exploring what various key bindings do.

Contributor

krader1961 commented Dec 27, 2016

Added commandline -f repaint to the bottom of the function.

Thanks. The reason you don't need it, and probably most people, is that you're using less or another pager that switches to the alternate screen by default. I don't like that behavior so I put --no-init in my $LESS env var. Which means less scribbles on the screen containing the prompt for me. I've never bothered to fix this because I've only used [alt]H a couple of times when exploring what various key bindings do.

@faho

faho approved these changes Dec 27, 2016

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Dec 28, 2016

Contributor

Merged as commit f9835b5. Thanks.

Contributor

krader1961 commented Dec 28, 2016

Merged as commit f9835b5. Thanks.

@krader1961 krader1961 closed this Dec 28, 2016

@faho faho added the release notes label Jan 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment