sshaw / git-link

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

Support for linking the file rather than the line #71

Closed husainaloos closed 4 years ago

husainaloos commented 4 years ago

Thanks for the very valuable package. I appreciate your work.

One issue I came across was that the package does not support opening the file in the browser. Typically, I have 3 cases when I open a file in the browser:

  1. Selecting a single line. This means that start=the selected line, and end=nil
  2. Selecting a region. This means that start=first line of the region, and end=last line of the region
  3. The file (with no selection). In such case, the start=line of the cursor, and end=nil

there is not way to distinguish 1 from 3. If I want to point a given file (no single region selected) to a collaborator, I won't be able to do that without losing (1).

I don't know if this is something you are interested in supporting.

sshaw commented 4 years ago

One issue I came across was that the package does not support opening the file in the browser.

One can open the file in the browser by setting git-link-open-in-browser but I assume you mean without a line number?

If I want to point a given file (no single region selected) to a collaborator, I won't be able to do that without losing (1).

Yes, linking out a line is something that would be nice to have, but what's the API for this?

I think using a prefix argument to git-link, git-link-commit, etc... is the best approach but currently the default prefix arg prompts for a remote. This is something I think should probably change in 1.0 but that aside what to do now? Maybe an additional prefix arg?

There is also the case to use a prefix are when one wants to enter a branch name.

husainaloos commented 4 years ago

That is correct. I meant the file without a line number.

Here is a suggestion. Though it might affect the behavior of handlers people use in git-link-remote-alist, it will not cause an error:

Currently any handler will have to handler the following arguments: (hostname dirname filename branch commit start end). We can have it where there are three cases for start/end values:

I believe this is the behavior of GBrowse in fugitive in the vim world.

sshaw commented 4 years ago

Hi,

We can have it where there are three cases for start/end values: ...

Yes, agreed. But how to tell the interactive git-link command to not generate the line number? Currently if it's used in cases like magit-status buffer there's no line number so it's not an issue but in a regular buffer with a point, how do say: don't use the line number?

talwrii commented 4 years ago

But how to tell the interactive git-link command to not generate the line number?

The "emacsy" way of doing this would be to use a prefix argument (e.g. press C-u a number of times before the command).

An alternative approach would be to use a dynamically bound environment variable and let people define their own keybindings if they want a file without a line (this is what I end up using myself most of the time).

(lamba () (intteractive) (let ((git-link-include-line nil)) (git-link))

Although actually, I think the best approach is to omit the line if you are line 1 (this is pretty intuitive and easy to remember).

sshaw commented 4 years ago

The "emacsy" way of doing this would be to use a prefix argument (e.g. press C-u a number of times before the command).

Indeed.

An alternative approach would be...

Yes, this is what I wanted to hear about, alternative approaches. But unless @husainaloos has any other ideas, I think the best option is to use a prefix arg of C-u - git-link. - is a good mnemonic for "without line number".

Although actually, I think the best approach is to omit the line if you are line 1 (this is pretty intuitive and easy to remember).

I sorta like this but linking to line 1 is a valid use case. As an innocent use I'd consider it a bug instead of a feature.

husainaloos commented 4 years ago

Hi,

We can have it where there are three cases for start/end values: ...

Yes, agreed. But how to tell the interactive git-link command to not generate the line number? Currently if it's used in cases like magit-status buffer there's no line number so it's not an issue but in a regular buffer with a point, how do say: don't use the line number?

@sshaw I am no emacs power user and I am not familiar with elisp enough to wrote lisp. So take what follows with a grain of salt.

I don't think I understand the difficulty. Maybe it is technical? Emacs has a function that determines if there is a selected region in the buffer or not. If there is no selected region, then the link generated only contains the file and no start/end line. If there is a region selected, then we calculated the start and end of that line, and generate a link based on that:

if (use-region-p) {
    start = line-number-at-pos(region-beginning)
    end = line-number-at-pos(region-end)
} else {
    start = end = nil
}
call hook_function(start, end)

Would that not work?

sshaw commented 4 years ago

Currently if there's no region we link to the line number. Can't stop doing that. I think the best bet is to check for a prefix argument of -. So assuming git-link is bound to C-c g l you'd type: C-u - C-c g l. = "minus the line number" 💲💲💲

Maybe there's a setting too git-link-no-region-no-line-number...

husainaloos commented 4 years ago

"Currently if there's no region we link to the line number. Can't stop doing that." Is the reason to maintain compatibility? Because to me, and I know this is subjective, opening a file when no region is selected is the more intuitive approach.

sshaw commented 4 years ago

"Currently if there's no region we link to the line number. Can't stop doing that." Is the reason to maintain compatibility?

Linking to a single line is a valid use case. I do it all the time. I don't think having to select a 1 line region just to link to that line is a good UI.

This repo is 6 years old and there has been a ton of requests. This is the first to not want line number at point.

sshaw commented 4 years ago

@husainaloos, @talwrii take a look at https://github.com/sshaw/git-link/commit/f1763182c9943f275ae1a15d7f33798bd88158a7.

sshaw commented 4 years ago

Added to v0.8.0.