Skip to content

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

Merged
merged 9 commits into from
Nov 14, 2017
Merged

Add line number interval setting #37120

merged 9 commits into from
Nov 14, 2017

Conversation

djweber
Copy link
Contributor

@djweber djweber commented Oct 30, 2017

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.

screen shot 2017-10-30 at 3 12 49 pm

screen shot 2017-10-30 at 3 14 59 pm

@alexdima alexdima added this to the November 2017 milestone Oct 30, 2017
@djweber djweber changed the title Feature/line number interval setting Add line number interval setting Oct 30, 2017
@wiktor-k
Copy link

wiktor-k commented Nov 2, 2017

Looks very good! Thanks.

Copy link
Member

@alexdima alexdima left a 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': {
Copy link
Member

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.

@wiktor-k
Copy link

wiktor-k commented Nov 8, 2017

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!

@djweber
Copy link
Contributor Author

djweber commented Nov 9, 2017

@alexandrudima I thought it would be acceptable to use the union type since there are several throughout the configuration settings, with editor.quickSuggestions being a good example:

https://github.com/Microsoft/vscode/blob/master/src/vs/editor/common/config/commonEditorConfig.ts#L355

As requested, I made it so that lineNumberInterval is no longer a union type.

@alexdima
Copy link
Member

The editor.quickSuggestions is a union type for backwards compatibility purposes and should not be used as a good example. It shipped for a couple of years as a boolean, and then someone in the team refined it.

@alexdima alexdima merged commit 0e0e347 into microsoft:master Nov 14, 2017
@alexdima
Copy link
Member

@DDWR Thank you for your contribution! ❤️

@elektronik2k5
Copy link

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.

@djweber
Copy link
Contributor Author

djweber commented Dec 22, 2017

@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?

@TrevorSayre
Copy link

@alexandrudima Is there any chance we can get back the ability to specify an interval?

@alexdima
Copy link
Member

alexdima commented Feb 5, 2018

@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.

@F1LT3R
Copy link

F1LT3R commented Feb 11, 2020

I would also like to see the interval configurable.
And it would be good to have an option for current only, eg:

{
   "editor.lineNumbers": "current"
}

@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants