-
Notifications
You must be signed in to change notification settings - Fork 32k
Add line number interval setting #37120
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
Add line number interval setting #37120
Conversation
Looks very good! Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For our users sake, who will have to figure out how to configure this setting, I would kindly ask that we remove the OR type for this setting... IMHO, it makes it too complicated.
@@ -207,6 +207,32 @@ const editorConfiguration: IConfigurationNode = { | |||
'default': 'on', | |||
'description': nls.localize('lineNumbers', "Controls the display of line numbers. Possible values are 'on', 'off', and 'relative'. 'relative' shows the line count from the current cursor position.") | |||
}, | |||
'editor.lineNumberInterval': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, the introduction of the OR type makes this setting too difficult to configure. It is technically correct, but does not align with the other editor settings.
I would remove the usage of the OR type.
I've fixed issues reported by @alexandrudima (splitting settings) on top of @DDWR's changes but as I can't push these changes to this PR. Should I create a new PR for that or would it be possible for maintainers to push my changes to this PR? Thanks in advance for help! |
@alexandrudima I thought it would be acceptable to use the union type since there are several throughout the configuration settings, with As requested, I made it so that |
The |
@DDWR Thank you for your contribution! ❤️ |
I would love to have a setting to control the line interval, as suggested here: #36981 (comment). The default 10 lines is too sparse, but using a configurable interval, I could set it to 5 or 2 to have just enough line numbers. |
@elektronik2k5 The work I did on this PR did have a configurable interval length, but for whatever reason it was completely changed to being a hard-coded interval of 10. I had not noticed that my PR was heavily modified like this and am pretty surprised that no discussion was made around it. @alexandrudima can you please clarify why you made these changes? |
@alexandrudima Is there any chance we can get back the ability to specify an interval? |
@dotweber @TrevorSayre It perhaps does not make sense, but I'm actively trying to add as little as possible to VS Code. And when something is added, I'm trying as much as possible to make it simple to use / configure / discover. The initial feature request at #36981 had 1 upvote, so AFAIK there were a grand total of 3 interested people in it. The feature request does not mention a configurable interval, so I assumed @dotweber is overdelivering. Plus the screenshots appear to use the value 10. Plus I honestly believe 10 is a good default, as the line numbers are presented in base 10. I am open to bring the possibility to configure the line numbers interval. But let's start with a fresh feature request (i.e. please create one); PR also most welcome. |
I would also like to see the interval configurable. {
"editor.lineNumbers": "current"
} |
This adds a setting to control the interval at which line numbers are rendered in the editor, described in #36981. You can specify a numerical interval as well as whether or not to show the line number for the current cursor position.