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

help/docs bug: blame view keybindings are hard to discover #1315

Open
ilyagr opened this issue Feb 11, 2024 · 4 comments
Open

help/docs bug: blame view keybindings are hard to discover #1315

ilyagr opened this issue Feb 11, 2024 · 4 comments

Comments

@ilyagr
Copy link

ilyagr commented Feb 11, 2024

I wish they were documented in the in-program help better. There could be a "blame bindings" section. I realize that all the bindings are mentioned in the help already, but their descriptions make it very difficult to understand how they actually work in blame view. Blame is very useful occasionally, but I find it difficult to remember the bindings whenever I use it.

My understanding is that, in blame view:

  • Enter opens the blamed commit for the current line in a separate pane
  • b opens the current file at the blamed commit when already in blame view
  • < goes back to the previous view, e.g. undoes b (as elsewhere in tig, but I only use it when blaming)
  • , seems like it could be useful according to the help, but is not actually useful in blame view AFAICT. UPDATE: It is useful, but I'm still unsure what exactly it does. See the next comment. UPDATE 2: It shows the last commit before the blamed commit for the current line that modified the selected line.

Thanks for making tig, it's very useful! I was thinking of making a PR for this, but I couldn't immediately figure out how the help is generated.

@ilyagr ilyagr changed the title blame view keybindings are hard to discover help/docs bug: blame view keybindings are hard to discover Feb 11, 2024
@ilyagr
Copy link
Author

ilyagr commented Feb 13, 2024

I realized that even here, I misunderstood something. It seems that the , command is important in blame. I'm guessing it's meant to go to either (the last commit before the current one that modified the current file) or (the last commit before the blamed commit for the current line that modified the selected line).

Update: It seems to be the latter. I wish there was a command for the former as well. E.g., , could do the former and B could do the latter. When I read the , in the help described as "Move to parent", I expected the former behavior.

Another confusing thing about the , command is that it sometimes shows the message "The selected commit has not parents". I wish this message was clearer: is the selected commit the one on the current line or the one currently being viewed? It could say something like "Commit abcdef1 has no ancestors that modify this line", that would already make things clearer.

@ilyagr
Copy link
Author

ilyagr commented Feb 13, 2024

BTW, I'd be happy to try implementing this myself, but it'd help if you gave me some pointers.

ilyagr added a commit to ilyagr/tig that referenced this issue Feb 13, 2024
This addresses a portion of jonas#1315.

Apart from being a bit clearer, the user will be able
to learn what `,` is meant to do from it. Trying to
press `,` a few times is a natural thing to try to
do after reading the help while trying to figure
out how blame works in tig.
ilyagr added a commit to ilyagr/tig that referenced this issue Feb 13, 2024
This addresses a portion of jonas#1315.

Apart from being a bit clearer, the user will be able
to learn what `,` is meant to do from it. Trying to
press `,` a few times is a natural thing to try to
do after reading the help while trying to figure
out how blame works in tig.
ilyagr added a commit to ilyagr/tig that referenced this issue Feb 13, 2024
The message is now:

   Commit f4657f0 has no ancestors modifying the seleted line

instead of  

   The selected commit has no parents

This addresses a portion of jonas#1315.

Apart from being a bit clearer, the user will be able
to learn what `,` is meant to do from it. Trying to
press `,` a few times is a natural thing to try to
do after reading the help while trying to figure
out how blame works in tig.
ilyagr added a commit to ilyagr/tig that referenced this issue Feb 13, 2024
The message is now:

   Commit f4657f0 has no ancestors modifying the seleted line

instead of  

   The selected commit has no parents

This addresses a portion of jonas#1315.

Apart from being a bit clearer, the user will be able
to learn what `,` is meant to do from it. Trying to
press `,` a few times is a natural thing to try to
do after reading the help while trying to figure
out how blame works in tig.
ilyagr added a commit to ilyagr/tig that referenced this issue Feb 13, 2024
The message is now:

   Commit f4657f0 has no ancestors modifying the seleted line

instead of  

   The selected commit has no parents

This addresses a portion of jonas#1315.

Apart from being a bit clearer, the user will be able to learn what `,`
is meant to do from it. In particular, they should notice that commit
`f4657f00` is the commit on the selected *line*, not the commit that is
currently displayed in general.

Trying to press `,` a few times is a natural thing to try to do after
reading the help while trying to figure out how blame works in tig.
@krobelus
Copy link
Contributor

, shows the last commit before the blamed commit for the current line that modified the selected line.

Right. I never really used, it seems nice. Sadly it can't know where a line moved to, so the initial position can be confusing.
I usually alternate between blame and diff view for recursive blaming.

ilyagr added a commit to ilyagr/tig that referenced this issue Feb 13, 2024
The message is now:

   Commit f4657f0 has no ancestors modifying the seleted line

instead of  

   The selected commit has no parents

This addresses a portion of jonas#1315.

Apart from being a bit clearer, the user will be able to learn what `,`
is meant to do from it. In particular, they should notice that commit
`f4657f00` is the commit on the selected *line*, not the commit that is
currently displayed in general.

Trying to press `,` a few times is a natural thing to try to do after
reading the help while trying to figure out how blame works in tig.
@ilyagr
Copy link
Author

ilyagr commented Feb 13, 2024

Sadly it can't know where a line moved to, so the initial position can be confusing.

That's a good point. If the "going to an ancestor of the currently displayed commit" operation existed, it'd be equivalent to b followed by that operation, so it overall seems less important.


I think having b, B (optional, would do what , currently does), , (for going to a parent of the currently displayed commit, as seems to match its description in the help, and is nice to track the evolution of a file), and < would be a good interface, especially it's documented. The (new) blame section of the help could document b and B, and either also document , and < (repeating an earlier section, possibly with variations), or just mention "See also , and < commands which are particularly useful in blame view".

We could also keep , as is, and add B that does the other thing, but I think that's be more confusing for all people except those who are very used to the current behavior of ,.

For diff view, I get a bit confused by the multi-view controls, so I seem to tend to avoid it. I just realized that <Tab> seems to switch the meaning of j/k and J/K, but not the meaning of Up/Down (Update:) or J/K. But that's a separate issue that might be harder to address.

Update:

I usually alternate between blame and diff view for recursive blaming.

I'm realizing I'm not sure how to do this,

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

No branches or pull requests

2 participants