gitlab.nvim icon indicating copy to clipboard operation
gitlab.nvim copied to clipboard

Ranged comments don't set range correctly

Open jakubbortlik opened this issue 11 months ago • 2 comments

Bug Description

The line range of a comment is not properly applied in Gitlab. If I create a ranged comment in Gitlab online, it actually contains the range information (it's the second comment in the screenshot below).

The missing range information is not just cosmetic it also makes it impossible to properly insert a suggestion in the gitlab.nvim "ranged" comment. If I want to modify it and click "Insert suggestion" it pastes this:

```suggestion:-0+0
abc
```

Instead of the correct:

```suggestion:-1+0
defghi
abc
```

The problem is that setting this correctly from within gitlab.nvim may currently not be possible due to limitations of the Gitlab API and of the go library that only seem to allow setting the line code and line position type ("old" vs "new").

Screenshots

First image shows a comment created in gitlab.nvim, the second one is directly from Gitlab: Image

jakubbortlik avatar Feb 19 '25 10:02 jakubbortlik

I've opened a MR in client-go that will enable setting notes like "Comment on lines 1 to 2" on discussions. I've tested in a local experimental branch of gitlab.nvim that this modification to client-go works.

However, the way how the LinePositionOptions are calculated in gitlab.nvim will have to be fixed, because currently, the plugin creates incorrect line ranges like this (-1 to -2 instead of 1 to 2): Image

One part of the issue is that for unchanged lines of code, the Type has to be the empty string, rather than "old" (I figured this out by comparing the data of discussions created in the plugin and in Gitlab.com). I suspect, there'll be more peculiarities like that and also that the incorrect way how the LinePositionOptions are currently prepared may also lead to errors like in #386.

I'll try to prepare a PR for this.

jakubbortlik avatar Apr 11 '25 23:04 jakubbortlik

client-go version 0.128 supports setting OldLine and NewLine in LinePositionOptions. So when #448 is updated and merged, it will be possible to move on with this issue (#478).

jakubbortlik avatar May 01 '25 22:05 jakubbortlik