Magit and Dired: fall back to current directory if not on file#110
Magit and Dired: fall back to current directory if not on file#110kisaragi-hiu wants to merge 1 commit intosshaw:masterfrom
Conversation
This is most noticeable in magit-status, where you'd have to visit a random file to be able to jump to the repository page with git-link. In Dired, it's also noticeable if git-link is run when point isn't over a file, for instance on Dired's current directory heading. This commit makes it so that, for example, running git-link in magit-status will fall back to visiting the repository at the current branch or commit. Then, as a special case, if current directory is project root (ie. `git-link--relative-filename` returns ""), visit the homepage instead. Now git-link in Magit and Dired always goes somewhere sensible, instead of returning an error.
| ((not (functionp handler)) | ||
| (message "No handler found for %s" (car remote-info))) | ||
| ((equal filename "") | ||
| (git-link-homepage remote)) |
There was a problem hiding this comment.
Thanks for the PR —with videos! 📼
Will have an in-depth look next week (please message if you do not hear from me!) but my initial thoughts on this line is that, all things being equal, this is a reasonable default but, one upgrading from existing versions could deem this a bug: "Link missing path". But, it is also possible I'm missing some broader context here that negates this.
There is also a plan for v1.0, where breaking changes will be make to existing defaults and this kinda thing may be more acceptable.
Thoughts?
There was a problem hiding this comment.
This is the special case for repository root, but this also ended up discarding branch / commit information.
The main motivator for me to put the special case in is that, at least for GitHub, an empty path after /blob (.../blob/main/) causes a file not found error instead of redirecting to /tree (as is done for eg. .../blob/main/filename).
It's probably better do it properly, though, and adapt the handlers to return the right url for this.
|
@sshaw Is this ok to merge? I as well came across the issue where running |
In magit-status it will link to a file; if you're on the file. I.e., Staged/Unstged files. You want to link to something has no link but a link is generated anyways. Some may call this a bug. Which looks like the same outstanding question as last time If you want to link to the homepage can you call |
Dired will work but yes, if you try to link to a parent directory outside the repo root then what is there to link to? I think a better default here would be the username page not the repos root because you're not trying to link to that: you're trying to link to the parent directory. Under what circumstances would one generate a "cannot link to error" then? |
This is most noticeable in magit-status, where you'd have to visit a random file to be able to jump to the repository page with git-link. In Dired, it's also noticeable if git-link is run when point isn't over a file, for instance on Dired's current directory heading.
This PR makes it so that, for example, running git-link in magit-status will fall back to visiting the repository at the current branch or commit. Then, as a special case, if current directory is project root (ie.
git-link--relative-filenamereturns ""), visit the homepage instead.Now git-link in Magit and Dired always goes somewhere sensible, instead of returning an error.
Before
before.mp4
After
..after.mp4