Use commit when branch is empty

[Miciah]

Pass nil to the link builder if the branch could not be determined.

git-link--branch returns nil when it cannot not determine the current branch. However, url-hexify-string was converting nil to the empty string. Consequently, some link handlers were using the empty string as the branch name instead of falling back to the commit.

[sshaw]

Hi, thanks for the PR.

This looks like it only happens when git-link-use-commit is nil. But with this PR we will use commit when settings say not to. Shouldn't an error be signaled instead?

[sshaw]

Also how do you end up with these links? By what's described in #14, by the repo being in a detached head state, or something else? Thanks.

[Miciah]

Yes, my repository was in a detached head state. I'm not sure what you mean. We want to use the commit instead of the branch if git-link-use-commit is true, and this PR doesn't change that unless I'm missing something, but we also want to use the commit if the branch could not be determined. The problem is that git-link--branch indicates that the branch could not be determined by returning nil for the branch value, and git-link passes the branch value through url-hexify-string which turns nil into "", which causes the handlers to do the wrong thing.

[sshaw]

but we also want to use the commit if the branch could not be determined.

That's the question. Do we? If git-link-use-commit is nil, the user doesn't want to use a commit, but we end up using the commit anyways (the obvious exceptions being when in magit-revision-mode, git-timemachine-mode, etc...).

The alternate would be to output an error message unless git-link-use-commit is t.

What do you think the best option is?

[Miciah]

I want git-link to open the link when the repository is in a detached head state, but I want it to use the branch if it can be determined. As I understand it, setting git-link-use-commit to t means git-link should use the commit even if the branch could be determined, right?

How about making git-link-use-commit a tristate, introducing 'as-fallback as a recognized value? (Naming suggestions welcome!) Then we would have the following behaviour:

branch	git-link-use-commit	behaviour
nil	nil	error
nil	t	use commit
nil	'as-fallback	use commit
not nil	nil	use branch
not nil	t	use commit
not nil	'as-fallback	use branch

Code would look something like this:

    (cond ((null filename)
       (message "Can't figure out what to link to"))
      ((null remote-host)
       (message "Remote `%s' is unknown or contains an unsupported URL" remote))
      ((and (null branch) (null git-link-use-commit))
       (message "Can't determine the current branch"))
      ((not (functionp handler))
       (message "No handler for %s" remote-host))
      ;; TODO: null ret val
      ((git-link--new
        (funcall handler
             remote-host
             (git-link--remote-dir remote)
             filename
             (if (or
                  (git-link--using-git-timemachine)
                  (eq git-link-use-commit t)
                  (and (eq git-link-use-commit 'as-fallback) (null branch)))
             nil
               (url-hexify-string branch))
             commit
             start
             end))))

[sshaw]

I want git-link to open the link when the repository is in a detached head state, but I want it to use the branch if it can be determined... How about making git-link-use-commit a tristate, introducing 'as-fallback

Something similar to this has been discussed. Essentially a DWIM function. I more or less have punted on this myself and decided to leave it for v1.0 which, would also address some of the shortcomings/bad decisions e.g., not defaulting to git-link-use-commit t, prompting for origin but not branch, getting URL programmatically without adding to kill ring etc... But, not sure when that will happen 😢

There's a DWIM implementation here. And I would be down to add it prior to a 1.0. I really think that given the code that currently exists (in repo and referenced PR) this would be somewhat straightforward to write.

Is this something you'd be willing to add?

[sshaw]

Closing due to lack of activity and what appears to be an impasse.