Get the current file commit link if pt not on hash

[kaushalmodi]

• This functionality if off by default for backward compatibility.
• Set the new variable git-link-commit-fallback-use-latest-commit to t to enable this.

Personally I have never needed to get commit link for a hash under point. But I do find it useful to jump to the commit details associated to the commit of the current file in buffer (especially when studying emacs sources).

Also fixed the quoting styles: `%s' instead of '%s' to make the rounded quotes render nicely with the upcoming emacs 25.1.

(Next PR is making git-link work with the emacs git source: git.savannah.gnu.org)

[kaushalmodi]

Just FYI, this PR is unrelated to https://github.com/sshaw/git-link/pull/29.

The changes in this commit allow me to visit the page showing the diffs of the last commit. I access the commit diff pages very often. But I have never come across the need to jump to the commit page of the commit hash under point. This PR tries to do the right thing.. if there's hash under point, get the link to that commit, else get link to the current commit.

[sshaw]

Just FYI, this PR is unrelated to #29.

Yes, I actually meant to address that after the cgit/org-mode/savannah issue.

The changes in this commit allow me to visit the page showing the diffs of the last commit....

This is good, I like it. But, considering what the function currently does, I think its purpose is becoming a bit muddled. In one case it's just a function with a return value and the other an event handler for a UI; with a global variable (git-link-commit-fallback-use-latest-commit) acting as the arbitrator.

It feels a bit like:

(defun handle-click-or-read-file (file)
  (if i-want-to-handle-click
    (handle-click file)
  (read-file file)))

Do you think that's a fair assessment?

What are the other ways to make it work? The obvious alternatives are 1) new function or 2) prefix arg.

Don't really like 1, but I think the new function would be for the old behavior, something like git-link-commit-at-point. With 2 I think it's almost the same as what you have now. Given my example function the var i-want-to-handle-click would become current-prefix-arg (or similar), which arguably is permissible elisp behavior :smile:. But I still think that good development practices (in this case single responsibility) trump common development practices used when developing in a particular language.

Thoughts?

Maybe this can be done as part of the git-link refactoring I've been wanting to do.

Also note that with your change, if the file has no remote, the following message is printed in the minibuffer: Unknown remote 'nil'.

[kaushalmodi]

Will think over the best approach, rebase and resend PR.

[kaushalmodi]

I now solve this in my config using this:

(defun modi/git-link-commit-dwim (remote)
  "Create a URL representing the commit for the hash under point
in the current buffer's GitHub/Bitbucket/GitLab/... repository. The URL will be
added to the kill ring.

If the word under point is not a valid commit hash, use the latest commit hash
of the file in the buffer.

With a prefix argument prompt for the remote's name. Defaults to \"origin\"."
  (interactive (list (git-link--select-remote)))
  ;; Replace the `word-at-point' function within `git-link-commit' with a
  ;; custom function that returns the latest commit hash of the file in
  ;; buffer if point is not on a valid commit hash.
  (cl-letf (((symbol-function 'word-at-point)
             (lambda ()
               (let ((word (thing-at-point 'word))
                     (valid-commit-regexp "[a-z0-9]\\{7,40\\}"))
                 (if (and word (string-match-p valid-commit-regexp word))
                     word
                   (git-link--last-commit))))))
    (git-link-commit remote)))

@sshaw Would you like to bake this "dwim" behavior in the package?

[sshaw]

Not sure about the left stuff but otherwise sure.

Also thinking that:

1. git-link-commit should return current commit
2. git-link-commit-at-point should do what git-link-commit does now

And of course DWIM would combine the two? Maybe there are DWIM other scenarios for those using magit, vc-mode, etc..?

[sshaw]

Of course git-link-commit would have to emit a warning about the future behavior change before altering its behavior.

[kaushalmodi]

Not sure about the left stuff but otherwise sure.

Of course, you don't need that if this is supported by the package. I am using letf in my config so that I can prevent re-writing the whole fn in my config just to change this one behavior.

And of course DWIM would combine the two? Maybe there are DWIM other scenarios for those using magit, vc-mode, etc..?

I would proposed keeping only one function so that people don't have to bind multiple keys. The "DWIM" functions should be made configurable.. people can add/remove functions to that as they like. Idea is that git-link-commit will loop those that fn list and keep on applying each function in order till it is successful..

Example:

• Try getting commit for line at point in magit status..
• Try getting commit for hash string at point..
• Try getting commit from git-timemachine-revision if that minor mode is active
• Try getting current commit for the file

Of course git-link-commit would have to emit a warning about the future behavior change before altering its behavior.

With the above proposal, the change will be transparent.

WDYT?

[sshaw]

Of course git-link-commit would have to emit a warning about the future behavior change before altering its behavior.

With the above proposal, the change will be transparent.

A warning would still be needed since currently we would return nil if point is not on a hash but new behavior would return current commit for the file.

WDYT?

:thinking:

If you don't here from me soon on this feel free to come back here and remind me. :sleeping_bed:

[sshaw]

Closing as it's old and we have no resolution.