[Enceradeira]

Opening a file in the reviewer that has a ranged discussion fails in the DiffviewDiffBufWinEnter hook:

gitlab.nvim: Error setting diagnostics: .../gitlab/actions/common.lua:268: attempt to perform arithmetic on local 'new_end_line' (a nil value)

GitLab can return an empty line_code for both ends of a discussion's line_range, e.g.:

range = {
  ["end"] = { line_code = "", new_line = 26, old_line = 0, type = "new" },
  start   = { line_code = "", new_line = 26, old_line = 0, type = "new" },
}

parse_line_code then returns nil, nil, and get_line_numbers_for_range does arithmetic on the nil values (both the old-SHA and the new-SHA branch). The same crash is reported in #587 via jump_to_reviewer.

Changes

• get_line_numbers_for_range falls back to a single-line range when a line code can't be parsed.
• parse_line_code no longer throws on a nil line code.
• Adds tests/spec/line_code_spec.lua, which reproduces the error without the fix.

🤖 Generated with Claude Code

[jakubbortlik]

Hi @Enceradeira, are these empty line numbers created by gitlab.nvim or by Gitlab in the browser? I'm asking because maybe this is not the right place to fix this. If the missing line numbers are caused by gitlab.nvim, then we should fix the line code creation.

[jakubbortlik]

By the way there is #587 - is it solving the same problem? The author of that other PR never replied to my question, so I didn't bother to merge it :)

[Enceradeira]

Hi @jakubbortlik, the empty line numbers don't come from gitlab.nvim. The Go server always builds line codes as %x_%d_%d from non-pointer ints (cmd/app/comment_helpers.go), so it can't produce an empty value.

I checked the failing discussion with <leader>p. It turned out the line codes weren't partially empty as I originally assumed, but completely empty:

range = {
  ["end"] = { line_code = "", new_line = 26, old_line = 0, type = "new" },
  start   = { line_code = "", new_line = 26, old_line = 0, type = "new" },
}

I'm sure the comment wasn't written in Neovim. It was written by a colleague who doesn't use it, most likely in the GitLab web UI. I could ask whether they used an Emacs plugin, if that's important.

I've updated the PR accordingly: get_line_numbers_for_range falls back to a single-line range when a line code can't be parsed, and parse_line_code no longer throws on nil. The specs now use empty line codes.

Regarding #587: yes, it's the same crash, reached via jump_to_reviewer. This PR has the same guard in get_line_numbers_for_range, plus the nil safety in parse_line_code and specs, so it should cover #587 as well.

I'm happy to investigate further if you tell me what you'd like me to look into.

[jakubbortlik]

Hi @Enceradeira, thanks for investigating this and updating the PR. If you want to take it a step further you could try to find out if we could use the range.{end,start}.{new_line,old_line} attributes as fallback if the line code is missing. The reason get_line_numbers_for_range only uses the old_line, new_line, start_line_code, and end_line_code parameters is that the old and new lines in the range.{start,end} hadn't been implemented yet when the function was written. Maybe it could get the complete range object as a parameter instead of the growing list of parameters that are mostly individual attributes of the range.

I've got a vague idea that Gitlab does some updating of the values when a file in a MR is modified and the line numbers change, but I haven't really tried to pin the changes down. So maybe there's an opportunity to not only fix the bug you've encountered, but to also improve the navigation for comments on lines whose line numbers have changed.

Maybe to fix this properly, we'd have to also parse the following comments for patterns like changed this line in [version 4 of the diff](/path/to/project/-/merge_requests/123/diffs?diff_id=123456&start_sha=51b66f5693d988f2e0ee2ecb417e1c4bcc6adc0b#51b66f5693d988f2e0ee2ecb417e1c4bcc6adc0b_107_107). In that case, that should become a separate PR. If you don't want to do that we can simply merge your fix PR. It adds value on its own.

[Enceradeira]

Hi @jakubbortlik,

I've pushed a commit that takes up your first suggestion: get_line_numbers_for_range now takes the whole range and falls back to range.{start,end}.{new_line,old_line} when the line codes are empty. It only falls back to a single line when the range spans deleted and added lines, where those values are 0 on one side.

Last week I also looked into tracking comments whose line numbers changed between diff versions, but I couldn't find my way around it as quickly as I'd hoped. I'm happy to try again in a separate PR.

In the meantime, as you suggested, I'd be grateful if you could merge this one. That also keeps the fix and the broader improvement in separate PRs.

Let me know if you'd like anything done differently.

[jakubbortlik]

Hi @Enceradeira, thanks for continuing with the PR. Please run stylua . and push again. I guess, it would be useful to set up pre-commit...

Let's merge this now to fix the current bug. Then I will be grateful if you could investigate a little bit more and see if get_line_numbers_for_range couldn't be fixed even more, or maybe simplified -- it's currently quite hard to read -- all the "old/new", "start/end" combinations...

One think to consider: get_line_numbers_for_range now first looks whether a comment has the old_line set - if it does, it will place the comment in old version of the file, even for comments on unchanged code. I'd like to change this so that comments are preferably placed in the new file, if possible. This will be useful if I ever get to finishing suggestions preview.

[jakubbortlik]

By the way, I've now noticed that the missing line codes come from comments created in Gitlab.com itself 😱

[jakubbortlik]

@Enceradeira, thanks for the contribution!