Relative Line Number support #12055
Relative Line Number support #12055
Conversation
|
Hi @alfanick, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
|
@alfanick, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
|
I've made some more aggresive optimizations on alfanick/vscode/optimized-relative-numbers – I'm assuming that there is no need for rendering line numbers on modelLinesInserted/modelLinesDeleted as cursorPositionChanged is always called with these events, also no need for rendering with every modelLineChanged unless wordWrapping is enabled (this is only case when line can break and change numbering). I'm not really expert on VSCode flow, so these optimizations actually may break smth, but they work for me – the pull request does not brake anything, where branch optimized-relative-numbers does not brake anything either, but needs thorough code review (I can merge optimized-relative-numbers into alfanick/vscode/master if requested). |
There was a problem hiding this comment.
Thank you!
The change gets correctly the adding an option code, and when to update/re-render the line numbers.
However, it appears to me that it breaks the old behaviour of non-relative line numbers completely.
Please address these comments, and resubmit the PR, as it appears many unnecessary commits have made their way into this PR.
Looking forward to your changes! 👍
| return false; | ||
| } | ||
|
|
||
| this._currentLineNumber = modelPosition.lineNumber; |
There was a problem hiding this comment.
In the case where this._relativeLineNumbers is false, this._currentLineNumber should still be assigned such that this._currentLineNumber always has a good value.
It is possible that the relativeLineNumbers option changes at runtime and in that case we'd want this._currentLineNumber to be up-to-date.
| let relativeLineNumber = position.lineNumber; | ||
|
|
||
| if (position.column === 1) { | ||
| if (this._relativeLineNumbers) { |
There was a problem hiding this comment.
The test if (this._relativeLineNumbers) should be the first thing done here before doing any position conversions. All the new rendering code should go in this if. All the old rendering code should be maintained in an else branch. As it looks right now, regular line number rendering appears to be broken.
| @@ -3,6 +3,7 @@ npm-debug.log | |||
| Thumbs.db | |||
| node_modules/ | |||
| .build/ | |||
| .vscode/.browse.VC* | |||
There was a problem hiding this comment.
Please remove this one, you can use .git/info/exclude for it
|
@alfanick Please let me know if this is something you want to look into, otherwise I will pick it up, as it is something we want to get done in Sept (i.e. by Friday the latest). Then I need to expose extension host API for it. |
|
I've implemented this in 13bbe32 |
Adds support for vim-style relative line numbers.
Useful for vim motion plugins, replacing extension "Relative Line Numbers" by extr0py.
editor.relativeLineNumbersoption (changeable without restart), default to false