Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Wrong" relative line numbers when code folded after latest release #157282

Closed
wraiford opened this issue Aug 5, 2022 · 11 comments · Fixed by #159236
Closed

"Wrong" relative line numbers when code folded after latest release #157282

wraiford opened this issue Aug 5, 2022 · 11 comments · Fixed by #159236
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders regression Something that used to work is now broken verified Verification succeeded
Milestone

Comments

@wraiford
Copy link
Contributor

wraiford commented Aug 5, 2022

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.70.0
  • OS Version: macOS Monterey 12.5

Steps to Reproduce:

  1. Enable relative line numbers in user settings ("editor.lineNumbers": "relative")
  2. Open a code file (TypeScript in my case).
  3. Code fold a function (I use cmd+k, cmd+[)
  4. Put the caret on the first line of the resulting fold.
  5. Look at the relative line of the end curly brace for the function. the relative line number is 1.

Screen Shot 2022-08-05 at 10 50 49 AM

This relative line number used to be the actual line count, ignoring the code folding. I read in the release notes that you are doing a lot of work with code folding, and I didn't know if you knew this was changed behavior. I can understand that some may want this new behavior, but there should be a setting for it. Maybe there is, but I can't find it!

It probably doesn't matter, but my use case is that I use VsCodeVim and I use the relative line numbers to yank or drop an entire function/class/whatever. But this isn't their problem directly though, and of course I tested this behavior with extensions disabled.

Thanks for your help.

@PieterBranderhorst
Copy link
Contributor

This change was deliberate and is separate from the recent work on folding ranges. See #138787

I have no comment on which way is preferable, just adding the link here to what caused this.

@thstkn
Copy link

thstkn commented Aug 6, 2022

(Just) like OP im using VSCode NeoVim and rely on relative line numbers to not be ignoring if code is folded or not. I sincerely hope this new behaviour will become optional as I've gotten SO used to this.

@lakardion
Copy link

lakardion commented Aug 6, 2022

+1
I was totally used to the relative numbering with vim extension. Folds should definitely not obscure relative line number of following lines

@wraiford
Copy link
Contributor Author

wraiford commented Aug 7, 2022

This change was deliberate and is separate from the recent work on folding ranges. See #138787

I have no comment on which way is preferable, just adding the link here to what caused this.

Thank you very much for the link @PieterBranderhorst .

I guess the difference between that post and mine is that I used double quotes when I said "wrong". Definitely is breaking behavior and should have been an option. I would say the implementors were unaware of the vim aspect of this, but the OP in that thread actually mentions it. So it's a real head scratcher why the existing (and long-standing) behavior was changed.

I strongly hope the team reconsiders this breaking change, since vscodevim has 4M downloads since 2015 - long before the OP claimed without double quotes that the relative line numbers were wrong.

And NOT AT ALL to challenge the utility of it, but I'm totally ignorant as to what is the use of relative line numbers skipping folding? Life is so awesomely immense and diverse, I'm sure it's just something I'm unaware of.

@william-bit
Copy link

I hope this new behavior should get a new option rather than breaking the existing behavior. New behavior breaks a workflow when using vscodevim in yanking and deleting the line.

@alexdima
Copy link
Member

This was an intentional bugfix done via #155970 . We initially added support for relative line numbers as this was a highly requested feature from folks coming over from vim, so when I saw #138890 (comment) (i.e. how vim skips over folded regions when counting relative line numbers), I considered #138787 to be a bug.

We can easily revert the change and go back to the previous relative line counting, but I'm honestly a bit confused here with regards to what folks really expect 🤔 . Is this relative line counting tweakable via a setting in vim?

@alexdima alexdima self-assigned this Aug 10, 2022
@alexdima alexdima added this to the August 2022 milestone Aug 10, 2022
@lakardion
Copy link

I see why it was considered a bug. Native vim (or at least nvim) does relative numbers the same way as the PR in question introduced.

The big difference however is that in nvim when there are folded sections, if you 4j in ig:
image
You would end up in TeachersList line ( folded lines were skipped)

Whereas with VsCode vim, with the new change you would end up opening the fold. That is, it would go down literally 4 lines down from the cursor.
Thus, there is no way for the user to actually get to the desired line quickly by using relative line numbers

So I guess the discussion would have to be whether the solving of this issue now it is a bug for VsCode vim?

@alexdima
Copy link
Member

I see! Thank you for clarifying. I will roll us back to the previous behaviour as default and maybe introduce a setting for the new line counting or remove it altogether.

@alexdima alexdima added bug Issue identified by VS Code Team member as probable bug regression Something that used to work is now broken labels Aug 11, 2022
@wraiford
Copy link
Contributor Author

@alexdima I am not a vim proper user, nor a neovim one. I started using vim in vscode several years ago. I tried vim and neovim, but I prefer vscode. So I don't know about vim.

But I would imagine if you want to delete a function that takes up 10 lines and it is folded and shows only 1 line in relative line numbers, then when you hit 1dj (delete 1 line down) it would delete the entire folded section and not just a single invisible line. If it doesn't do this, then it also would be poor IMO.

When I want to delete entire functions/classes/etc. which can be more than one screen in height, I will fold that section and look at the relative line number and use it in the command. This goes not only for d (deleting), but also for y (yanking/copying) and gc (commenting). Probably other commands as well.

This is integral and used many many many times per code session. There are other vim specific tricks for blocks and other things which I also use, but I use folding - often in combination with folding entire files to "skim" at a higher level. So it's already folded often and I just look at the relative line number and do it.

All of that said, I love all of the work y'all do on vscode making it the best out there (IMO). I could see if you're getting other vim proper people who are complaining that you would see it as one-sided and not requiring a setting flag. But I didn't know that they were calling for the change, and I would say instead of reverting it, you would add the setting flag and set it to whoever you think the majority is. Maybe you can ask the others who were asking for it the behavior of vim proper, if it deletes the entire fold when doing the commands. Because it doesn't in vscode, rather only the invisible lines. I'd be dumbfounded if that's really expected behavior in vim proper.

@aeschli aeschli removed their assignment Aug 23, 2022
@iautom8things
Copy link

iautom8things commented Aug 25, 2022

but I'm honestly a bit confused here with regards to what folks really expect

I just want to chime in that, to me, the original don't skip over folded content when counting relative line numbers is what I would expect.

🙏 Thank you for considering reverting the changes.

@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Aug 25, 2022
@lramos15 lramos15 added the verified Verification succeeded label Aug 26, 2022
@wraiford
Copy link
Contributor Author

wraiford commented Sep 2, 2022

Thank you @alexdima !!

@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders regression Something that used to work is now broken verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants