If in magit-revision buffer, git-link can open the commit

[FrostyX]

I think this better than printing an error that

Can't figure out what to link to

When in a magit-revision buffer, people most likely want to link the displayed commit. But this also allows to link a commit that the cursor is pointing to.

[sshaw]

Hi, thanks for the PR.

Currently in a magit-revision buffer it will link, depending on where you click call the git-link*. For example:

You can get a link to the commit by calling git-link-commit on a commit hash and links to the files (file in this case) by calling git-link on their name or diff. How to does this PR change that?

[FrostyX]

Considering your screenshot ... If I install a git-link from master and move the cursor to the exact place you have, then:

• M-x git-link will result in Can’t figure out what to link to
• M-x git-link-commit will result in Point is not on a commit hash

Which on one hand makes sense and basically tells me that I need to move my cursor on some commit hash and run M-x git-link-commit again. On the other hand, it was quite obvious what I wanted to do. I am in a magit-revision, I ran git-link-commit, so I clearly wanted a link for that very commit.

In the PR I am improving git-link-commit to support this. At the same time, it is only a fallback. If the cursor points to some hash, it will open that hash. The new behavior only applies if the cursor doesn't point to a hash and git-link-commit would previously result in an error.

and links to the files (file in this case) by calling git-link on their name or diff.

I did not know that, thank you. That's a nice feature and my PR accidentally breaks that. What I was trying to achieve was ... if in a magit-revision buffer with a cursor pointing either to a commit hash or being anywhere in the commit header section, I wanted git-link to behave like git-link-commit and open the commit, instead of erroring out.

[sshaw]

In the PR I am improving git-link-commit to support this. At the same time, it is only a fallback. If the cursor points to some hash, it will open that hash. The new behavior only applies if the cursor doesn't point to a hash a

Great, thanks for clarifying. I wanted to understand what is going on high-level before lookin' at the code (too much! ).

...and links to the files (file in this case) by calling git-link on their name or diff.

...That's a nice feature and my PR accidentally breaks that. What I was trying to achieve was ... if in a magit-revision buffer with a cursor pointing either to a commit hash or being anywhere in the commit header section

Yes, sounds useful.

Currently the PR modifies git-link but sounds like it should be modifying git-link-commit? If so then that could fix the current breakage that you refer to.

[sshaw]

Added a Magit Revision Buffer section to the docs d9b375f

[FrostyX]

Added a Magit Revision Buffer section to the docs d9b375f

Nice, I think it will be helpful :-)

Great, thanks for clarifying. I wanted to understand what is going on high-level before lookin' at the code (too much! ).

My apologies, when writing the comment I realized I didn't explain the change in the PR description clearly enough.

If so then that could fix the current breakage that you refer to.

I think the change was almost correct, I just improved the condition. Please take a look, I think it works exactly how we want now.

[sshaw]

Thanks for the update.

Is there a need to modify git-link? From your description it sounds like we only need to modify git-link-commit

[FrostyX]

It wasn't necessary. I thought of it as a nice helper, allowing to just use git-link instead of having to remember git-link-commit but it is not what this PR was really about, so I dropped it :-)

[sshaw]

This function does not exist for me:

setq: Symbol’s function definition is void: magit-buffer-revision

I'm on Emacs 29.4 with Magit 20241001.2052. We should make sure it works on old versions (assuming this is the problem)

[sshaw]

I'm on Emacs 29.4 with Magit 20241001.2052. We should make sure it works on old versions (assuming this is the problem)

If this function was introduced in later versions and there's no alternative we can call (functionp) for the time being to guard against cases where it does not exist.