Questionable behavior for open_file_url (GitBlameOpenFileURL)

[nrako]

The current behavior of open_file_url (:GitBlameOpenFileURL) is opening the current file at the latest commit. However, most of the time the latest commit is not yet pushed to the remote web origin and this just open a 404 page. Making this command rather useless and confusing.

Since the primary purpose of git-blame is to preview git message on every line (a git blame peek summary) I'd expect that open_file_url would use the current commit previewed instead of the latest commit. This could eventually fallback to the current branch if the current line is not committed or if multiple lines were selected (if that were supported).

git-blame.nvim/lua/gitblame/init.lua

Lines 506 to 515 in 1f057f0

	local function open_file_url(args)
	local filepath = utils.get_filepath()
	if filepath == nil then
	return
	end
	get_latest_sha(function(sha)
	git.open_file_in_browser(filepath, sha, args.line1, args.line2)
	end)
	end

[f-person]

I see; these are valid concerns, IMO. Thank you for the issue!

---

@bossley9, any thoughts on this? Would implementing the suggestion (open at current line commit) be an issue in your workflow?

---

Maybe it'd make sense to add a configuration option for the latest commit behavior 🤔 so that we have a default (current line commit) but still allow it to be modifiable without introducing new commands.

[bossley9]

That's a valid concern and I think that makes sense! The main concerns I see with this are

1. It becomes a bit tricky when ranges are selected to view - which commit should be opened?
2. A user might still not have pushed the current line commit to the remote

I was thinking there could be a fallback to try and get the latest remote commit and view with that instead, but it might lead to some invalid ranges (local remote has a file with 300 lines but the remote still only has 100 lines and selecting a line or range from the new commits would lead to unexpected behavior). Not sure what should be done in these scenarios 🤷

[bossley9]

@nrako thoughts on this? I think it would make sense for git-blame to open to the current line commit, but you could still run into some 404s depending on what commits are pushed to the remote

[nrako]

@bossley9 – I didn't knew range selection were supported, it would be nice if the commit summary msgs would appear for every selected line after a different delay (higher by default); but I digress that's a different topic.

To answer your question, and as vaguely hinted in my initial msg, in both cases (range selected & lines not yet committed) the ref could try to fallback to the upstream branch. This would simply fail if there's no upstream branch and of course the lines could also be out of sync if the upstream branch is behind the local branch. But imho that'd still be more aligned with the "blame" context of this plugin and still somehow be a more resilient fallback than opening the file on the "last commit".

To try getting the current upstream branch something like that should work: git rev-parse --abbrev-ref --symbolic-full-name @{u}

For anything more complex we could start to question wether such features starts to overlap with plugins such as:

• https://github.com/ruifm/gitlinker.nvim
• https://github.com/linrongbin16/gitlinker.nvim
• https://github.com/tyru/open-browser-github.vim
• https://github.com/dinhhuy258/git.nvim

I don't use them as I'd rather only have something simple that go straight to the "blame" context for the selected line(s) but there might be merits of considering delegating such features to other plugins.

[nrako]

Actually I might start using https://github.com/linrongbin16/gitlinker.nvim as this is the closest to what I'd want; it supports range selection and check the existence of the upstream branch and the file on it. For git-blame.nvim I'd prefer that the context is on "Blame" instead of "Code", but this is only a click away with gitlinker.nvim so I'll use it for now.

There's probably some nice clues of how an "Open Github Blame" could work in that plugin too:
https://github.com/linrongbin16/gitlinker.nvim/blob/master/lua/gitlinker/git.lua

[bossley9]

You bring up some great points. Let me make sure I understand correctly, because this might be how we move forward with these command(s) in the future:

• if a single line is invoked with GitBlameOpenFileURL, we want to open the current blame commit for that line if it exists upstream. Otherwise, try to open the latest upstream commit.
• if a range of lines are invoked with GitBlameOpenFileURL, we want to open the blame commit for the most recently updated selected line if that exists upstream. Otherwise, try to open the latest upstream commit.
• out of scope but preferred: when a range of lines are selected, display the blame for all selected lines, not just the line the cursor is on.

Is this desirable behavior for your purposes @nrako? I think these changes make sense for the project as a whole. I'm not sure how feasible implementing some of these changes are (there will likely be some difficulty/performance issues querying the remote, but we'll see!) but I want to make sure the best solution is implemented. The "last commit" behavior you mentioned is relatively new behavior, and I'm open to suggestion on it 🙂

[nrako]

the latest upstream commit

I don't think it need to find the "latest upstream commit", this can simply goes to the upstream branch tree:

e.g with Github: https://github.com/$user/$repo/blame/$UPSTREAM_BRANCH/file.ext#L...

The plus side with this approach is that the user can realize that upstream is beyond and could push and refresh the webpage to see what he was expecting to see...

the blame commit for the most recently updated selected line if that exists upstream

I didn't thought that far, but that could actually be a nice thing.

[nrako]

user can realize that upstream is beyond and could push and refresh the webpage to see what he was expecting to see

Well actually, this makes me think that always trying to show the expected SHA could also be a desired behavior, especially for "blame" context, as a 404 becomes an indication that the user needs to push but then will be able to browse the code from the expected commit.

Just food for thoughts, up to you.

[bossley9]

Yeah, it's really a matter of whether a 404 would be expected or not. In that case, let's remove the fallback behavior and let it behave as follows:

• if a single line is invoked with GitBlameOpenFileURL, we want to open the current blame commit for that line.
• if a range of lines are invoked with GitBlameOpenFileURL, we want to open the blame commit for the most recently updated selected line.
• out of scope but preferred: when a range of lines are selected, display the blame for all selected lines, not just the line the cursor is on.

For all of these, we do not check if it exists upstream, we just provide the behavior and it would be up to the user to update their remote accordingly. These rules would also apply to GitBlameCopyFileURL except that the URL would be copied to clipboard. On a similar note, I believe we would want to do something similar for GitBlameCopyCommitURL, GitBlameCopyCommitSHA, and GitBlameOpenCommitURL to work with ranges as well (I believe these functions do not work with visual selections yet). How does this sound?

[f-person]

• if a single line is invoked with GitBlameOpenFileURL, we want to open the current blame commit for that line.
• if a range of lines are invoked with GitBlameOpenFileURL, we want to open the blame commit for the most recently updated selected line.

Yeah, I like this behavior! I was thinking of using the latest (remote) for the second scenario, but using the most recent one makes more sense.

---

I don't think we need to implement visual selections for GitBlameCopyCommitURL/SHA since these are related to a particular single commit. Also, what would be copied in that case? It does make sense for the file open/copy API to support ranges since it's something that's already in many use cases (and intuitively expected).

[bossley9]

Good point, that's true. Ok, we'll just stick with GitBlameCopyFileURL and GitBlameOpenFileURL. Thoughts @f-person @nrako ?

[f-person]

Sounds good to me!

[nrako]

sounds great to me!

[bossley9]

Awesome! I'll look into this in the next few days. I've also moved the other request into a new issue #106 so we can track it and hopefully work on it in the near future 🙂