sshaw / git-link

Emacs package to get the GitHub/Bitbucket/GitLab/... URL for a buffer location
399 stars 74 forks source link

try use tag/commit as branch when detached #67

Closed jiacai2050 closed 4 years ago

jiacai2050 commented 4 years ago

get current tag with reference to https://stackoverflow.com/a/18660163/2163429

I use golang/tools to test:

sshaw commented 4 years ago

Hi, thanks for this. Good idea.

So without your commit and without setting git-link-use-commit to t, you were getting an empty path segment, correct e.g., some//path/foo?

I think this could be turned into a feature/variable that instructs git-link to always use a tag. Any desire to try this? If not no problem, just please confirm the above path issue for me and I will merge.

If the path issue did not occur for you, then I think the need for the "use tag" variable is stronger.

Thanks a lot!

jiacai2050 commented 4 years ago

So without your commit and without setting git-link-use-commit to t, you were getting an empty path segment, correct e.g., some//path/foo?

Yes, it looks like this https://github.com/golang/tools/blob//internal/lsp/mod/hover.go#L42

But you remind me to deal with detached state without any tag on it, if then, just use commit.

sshaw commented 4 years ago

So without your commit and without setting git-link-use-commit to t, you were getting an empty path segment, correct e.g., some//path/foo?

I've created #68 to address this.

But you remind me to deal with detached state without any tag on it, if then, just use commit.

This is not needed. We only want to use the commit if git-link-use-commit is set to t.

This is how I think the git-link function should behave:

  1. Fix #68
  2. Add a variable git-link-use-tag defaulting to nil
  3. Look up branch as we are now
  4. Look up commit as we are now
  5. Look up the tag here
  6. Given ~a detached head~ any state:
    1. If git-link-use-commit is nil and git-link-use-tag is nil signal an error
    2. Else use whatever is appropriate based on the above variables

But with the variable git-link-use-tag, this should always link with a tag when set. Agree?

In a future version git-link-use-commit will default to t. Then you will not have these problems by default. It was a mistake to not make this the default from v0.0.1.

sshaw commented 4 years ago

Assuming you agree, would you be okay with modifying the PR to work with steps 2, 5, and 6?

jiacai2050 commented 4 years ago

I don't think we should raise an error in step 6.1. From users' points of view, this package should work out of box wihout any configuration, they can set git-link-use-tag to t if they want permalink, but when HEAD is detached, this package should try to guess what is most suitable here, and use tag or commit id as fallback(with tag preferred).

But with the variable git-link-use-tag, this should always link with a tag when set. Agree?

I agree with this. However if no tag is attached on HEAD, git-link should use branch/commit as fallback (with branch prefered).

If both git-link-use-tag and git-link-use-commit are set to t, then try tag first, fallback to commit when failed.

Any thoughts?

sshaw commented 4 years ago

Hi, thanks for the feedback.

I don't think we should raise an error in step 6.1. From users' points of view, this package should work out of box without any configuration,

I mostly agree, which is why at some point, i.e., v1, git-link-use-commit should default to t. But from here on let's assume we're talking about a v1.

but when HEAD is detached, this package should try to guess what is most suitable here, and use tag or commit id as fallback (with tag preferred).

If someone has git-link-use-commit set to nil it means they don't want a commit SHA to be used in a link. If a link is then generated with a commit, to me, this is a bug. Same applies to git-link-use-tag being nil and getting a link with a tag.

But with the variable git-link-use-tag, this should always link with a tag when set. Agree?

I agree with this. However if no tag is attached on HEAD, git-link should use branch/commit as fallback (with branch preferred).

Branch preferred by who? This is where the options come in. People have different preferences and one may not like the generated link if we don't respect their preferences/settings.

If both git-link-use-tag and git-link-use-commit are set to t, then try tag first, fallback to commit when failed. Any thoughts?

While it does make things more complex implementation-wise, maybe the solution is to add, in addition to git-link-use-tag:

  1. git-link-preferred-format which could be a list from most to least preferred: (tag commit branch) or similar
  2. git-link-no-matter-what which if t will disregard all settings and attempt to generate a link based on git-link-preferred-format.

Though git-link-preferred-format maybe could replace git-link-use-commit, etc... Though I'd probably keep and deprecate them until a v2 (if there's ever a v2).

jiacai2050 commented 4 years ago

I like git-link-preferred-format idea, the value can be a list of function, then users can return what they want in the url.

I push another commit, please take another look.

sshaw commented 4 years ago

Hi, thanks. Sorry for the delay. It's a bit of a pain to test this across all supported services. The tests cases are non existent here :(

Nevertheless a few things:

jiacai2050 commented 4 years ago

Hi, maintaining compatibility can be a pain thing, and I have taken carefully not to break any existing options. Please take another look, I have fixed the Azure issue.