Skip to content

fix: resolve rev revspecs, and separate the file-change warning from message - #306

Merged
linrongbin16 merged 2 commits into
linrongbin16:masterfrom
tummetott:feat/resolve-rev-and-split-file-change-warning
Sep 30, 2026
Merged

linrongbin16 merged 2 commits into
linrongbin16:masterfrom
tummetott:feat/resolve-rev-and-split-file-change-warning

Conversation

@tummetott

Copy link
Copy Markdown
Contributor

The problem

GitLink does not link to the commit you have checked out. When rev= is omitted, the rev comes from get_closest_remote_compatible_rev (lua/gitlinker/git.lua:276), which begins with:

-- try upstream branch HEAD (a.k.a @{u})
local upstream_rev = _get_rev("@{u}", cwd)
if upstream_rev then
  return upstream_rev
end

Consider the ordinary workflow: branch off main, commit some work locally, do not push yet. Those commits exist nowhere upstream, so there is no correct permalink for them. Instead of saying so, the fallback walks until it finds a commit the remote does have, and returns a URL built from that one.

The result is the worst possible failure for a permalink. The URL resolves. It opens a real file at real line numbers. The code shown is simply not the code you were looking at, because it comes from a different commit. Nothing about the link tells the recipient this, and often nothing tells the author either.

Passing rev= explicitly does not help, because the value is written into the URL verbatim. rev=HEAD produces /blob/HEAD/..., which forges resolve server-side to the default branch tip, landing you somewhere different again. Only a literal commit ID works today, which means computing one yourself before every call.

What this PR changes

1. rev= accepts any revspec. A caller-supplied rev is now resolved through git rev-parse before it is used, in make_linker.

The effect is that rev=HEAD produces a permalink to the commit you actually have checked out, and rev=<branch> or rev=<tag> resolve to the commit they currently name rather than being written into the URL as a moving ref. Anyone who wants strict permalinks can bind GitLink rev=HEAD and get a 404 on unpushed work, which is a failure that describes itself, rather than a link to unrelated code.

This also removes a re-assignment of lk.rev in _link. The same value is already handed to make_linker a line earlier, so it was a no-op before and would have thrown away the resolved commit ID after this change.

2. The file-change warning is its own option. A new config key, warn_on_file_change, defaults to true and can be overridden per call in the same way as message.

Printing the permalink on every copy is noise, so plenty of people set message = false. A warning that the link may not match what you are looking at is not noise, and it should not disappear with the echo. It now appears only when it applies, and is configurable on its own.

It is also raised at WARN level rather than the default, so notification plugins colour and route it accordingly, and it names the rev it was generated against.

Compatibility

rev=<full commit ID> behaves exactly as before, since a commit ID resolves to itself.

Users with message = true see the same information, now as a separate warning line rather than a suffix on the URL.

Users with message = false start seeing a warning they previously did not. That is the intent of the change, but it is a visible difference in default behaviour and worth a maintainer's judgment.

Comment thread lua/gitlinker.lua Resolved
Comment thread lua/gitlinker.lua Resolved
@tummetott

tummetott commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor Author

Both changed

@linrongbin16
linrongbin16 merged commit 693bc4f into linrongbin16:master Sep 30, 2026
4 of 5 checks passed
tummetott added a commit to tummetott/gitlinker.nvim that referenced this pull request Oct 1, 2026
PR linrongbin16#306 also removed a re-assignment of `lk.rev` in `_link`, and my description
presented that as a no-op cleanup, so it read as something separable. You asked
to move it to its own PR, I dropped it from the branch without re-testing, and
the PR was merged in that state. Going back over it afterwards, the removal is
required by the rest of linrongbin16#306 rather than being an independent cleanup.

`make_linker` resolves a caller-supplied `rev` through `git rev-parse`, then
`_link` re-assigns `lk.rev` from `opts.rev` afterwards, so the router still
receives the revspec as the caller typed it. `rev=HEAD` generates
`/blob/HEAD/...`, which GitHub resolves to the default branch tip, and
`rev=<branch>` writes a moving ref into the link. The resolved commit id
survives only inside `make_linker`, where it feeds the `is_file_in_rev` and
`file_has_changed` checks.

So url generation behaves as it did before linrongbin16#306. Removing the three lines is
what makes the merged change take effect.

`rev=<full commit id>` is unaffected, since `make_linker` resolves a commit id
to itself.
@tummetott tummetott mentioned this pull request Oct 1, 2026
linrongbin16 pushed a commit that referenced this pull request Oct 2, 2026
PR #306 also removed a re-assignment of `lk.rev` in `_link`, and my description
presented that as a no-op cleanup, so it read as something separable. You asked
to move it to its own PR, I dropped it from the branch without re-testing, and
the PR was merged in that state. Going back over it afterwards, the removal is
required by the rest of #306 rather than being an independent cleanup.

`make_linker` resolves a caller-supplied `rev` through `git rev-parse`, then
`_link` re-assigns `lk.rev` from `opts.rev` afterwards, so the router still
receives the revspec as the caller typed it. `rev=HEAD` generates
`/blob/HEAD/...`, which GitHub resolves to the default branch tip, and
`rev=<branch>` writes a moving ref into the link. The resolved commit id
survives only inside `make_linker`, where it feeds the `is_file_in_rev` and
`file_has_changed` checks.

So url generation behaves as it did before #306. Removing the three lines is
what makes the merged change take effect.

`rev=<full commit id>` is unaffected, since `make_linker` resolves a commit id
to itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants