microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.11k stars 29.27k forks source link

Distinguish code area from glyph area #3803

Closed anseki closed 8 years ago

anseki commented 8 years ago

This is small issue for me.

Code folding is very great useful. But it made margin between line numbers and first character in each line. When I use "2 spaces indent", that margin looks like indent. Now, I use editor.rulers setting includes 0 like [0, 100, 120] to indicate a start point of a line. But it is not pretty.

Is this issue solved by any theme? I think that it is solved by just a little changing a color of the left side area of first character in each line.

kumarharsh commented 8 years ago

+1. Not sure if this can be solved by theming. But if not, then definitely something the team should look at

mattacosta commented 8 years ago

Yeah I keep trying to double check that my code doesn't have extra spaces. It's mildly irritating. Before the text was close enough to the line numbers that it was obvious where the code area was.

aeschli commented 8 years ago

Assigning to @bgashler1. @alexandrudima FYI

kumarharsh commented 8 years ago

@aeschli I'm working on a PR right now with minimal changes

kumarharsh commented 8 years ago

@aeschli when I'm trying to commit my changes, the precommit hooks is showing this error:

src\vs\workbench\browser\media\vs-dark-theme.css: Missing or bad copyright statement
src\vs\workbench\browser\media\vs-theme.css: Missing or bad copyright statement

Hygiene failed with 2 errors. Check 'build/gulpfile.hygiene.js'.

I've not touched the copyright statements at all. Looking at the hygiene file, it seems that it'll throw that error when a copyright is present. How do I commit the file then? Do I have to remove the copyright?

aeschli commented 8 years ago

I can't reproduce. I added a line vs-dark-theme.css, created a commit. Works. Make a change to the copyright statement, create a commit, Fails.

Make sure you have also have no unstaged changes that modify the copyright.

kumarharsh commented 8 years ago

OK, I found the bug. Are you testing on a non-Windows system?

You're creating the copyright string like this: https://github.com/Microsoft/vscode/blob/master/build/gulpfile.hygiene.js#L93

The parts are joined with a '\n', not a '\r\n' for windows. So, when the files are being read, the strings are not equal, even though they exist.

kumarharsh commented 8 years ago

I've added a PR for fixing that hygiene check in #3834

jeffmcaffer commented 8 years ago

would very much like this fixed. I'm forever moving left but ending up at the end of the previous line. Very disorienting. Thanks @kumarharsh

bgashler1 commented 8 years ago

Is this a problem only because the tab space size is small (e..g 2 spaces)? I've personally never had a problem with this, possibly because I'm using a larger tab space size.

This is what it looks like to me while I'm hovering over the glyph area.

screen shot 2016-03-08 at 11 45 54 am
Tyriar commented 8 years ago

I've had this problem as well, the gutter has fooled me into thinking it was an indent. Most other editors use a darker gutter to distinguish the two.

Tyriar commented 8 years ago

And yes, I'm typically using 2 space indenting when this happens as well.

jeffmcaffer commented 8 years ago

Is the following indented? If you hit left arrow, will you end up one space to the left or at the end of the previous line?

folding

anseki commented 8 years ago

@jeffmcaffer, my temporary solution: editor.rulers setting includes 0 like [0, 100, 120] to indicate a start point of each line s01

egamma commented 8 years ago

Tagging for March to consider/discuss for March.

bgashler1 commented 8 years ago

@stevencl suggested the following:

Reducing the width and adding the line number highlight sounds like it would help.

One of the commenters posted a screenshot asking what would happen when pressing the left arrow. With the line highlight it would look something like this:

image005

jeffmcaffer commented 8 years ago

@anseki, that actually works pretty well and interestingly is (appearance-wise) exactly what I imagined a solution to be. Perhaps having 0 in there should be a default?

@bgashler1 the line number highlighting might help for navigation the ruler line gives a continuous sense of nesting/indentation.

bgashler1 commented 8 years ago

@jeffmcaffer. Thanks for the feedback, for the sake of discussion, here are 3 additional ideas that may address the "continuous sense of nesting" you mentioned...

1) Small gutter width until hover.

Then gently transition the width up to allow for the folding icons (move the code to the right temporarily). We could also incorporate the vertical folding braces from Visual Studio, which may serve somewhat as a left-edge guide and will also clarify what regions will fold. distinguish_gutter_movecode

2) Make code folding affordances persistent.

No hover required in gutter to have them appear. comparison We could make vertical folding braces a little more low-key if they are too bright or loud.

3) Use a gutter style

Such as a background color or border

gutter background

background-color: rgb(37, 37, 37)

Or...

outline copy

border-right:rgb(51, 51, 51) solid 2px

4) Highlight the current line number to show edge of gutter. Decrease gutter width.

image

This is a more conservative, leaner design choice because it avoids a big line ruler or a heavy background color as used in option 3.

Sublime also does this approach, and we haven't heard anyone complain about it.

mattacosta commented 8 years ago

Well, whatever you guys end up choosing, you might want to shift the text by 1px or so. It makes option 3 look odd to me and causes text to overlap the selected line box too. line_overlap_14pt

Tyriar commented 8 years ago

The first option in option 3 looks the best to me, maybe giving 1-2px left-padding to the text area as @mattacosta mentioned. Not sure which shade works best, it needs to look good across themes. You can do it with a tmTheme in Sublime apparently.

jeffmcaffer commented 8 years ago

My 2c is for the line (second option in #3). shading has a habit of not working well across monitors, projectors, viewing angles, ... The line is simple and well understood IMHO

anseki commented 8 years ago

@bgashler1 I hope option 3-1 (background-color) because:

For example, this is indented lines without nesting: s01

This is nested: s02

Yes, indent lines without nesting might be strange, but that strange indent is not found. I mean, it should work without depending on the code folding.

anseki commented 8 years ago

Sorry all, my English is very poor... Can you understand my words?

kumarharsh commented 8 years ago

I agree with @anseki that there should be at least a visual separation for the gutter. As an added plus, option 1 seems very enticing on paper (or rather, in the GIF).

The PR I've made enables the 3rd option - but right now it looks... not good... because the gutter border is sticking very near to the code, like in the mocks here. I'd love to improve that, but it seems I'll have to modify the monaco-editor code which puts some styles inline onto the gutter element, and it's a little daunting going through the whole typescript code. Some pointers as to where to look would help :)

bgashler1 commented 8 years ago

@anseki I understand you perfectly. Thanks for sharing your feedback. @kumarharsh thank you as well. I'll pass this feedback on to the team and update the issue soon.

jeffmcaffer commented 8 years ago

FWIW, option 1 is looks cool but would IMHO be quite distracting in practice. having the code shifting around is like playing whack-a-mole. 3.* is simple, clean and easy. 3.2 seems particularly robust. Perhaps with a 1-2 pixel shift right of the text or shift left of the line. In the meantime, the rulers suggestion by @anseki in the first comment is working fine for me.

bgashler1 commented 8 years ago

Added one more option to my comment above (Option #4).

4) Highlight the current line number to show edge of gutter. Decrease gutter width.

image

This is a more conservative, leaner design choice because it avoids a big line ruler or a heavy background color as used in option 3.

Sublime also does this approach, and we haven't heard anyone complain about it. image

jeffmcaffer commented 8 years ago

Having just the current line/selection's left margin illustrated seems too narrow. When the user is cursoring around fast and looking ahead, they might not be at the line they are looking at. As such they will have no point of reference for where to go. Seems like driving with narrow or short range headlights.

chrisdias commented 8 years ago

@jeffmcaffer i'm curious, do you have this problem in sublime?

Tyriar commented 8 years ago

I used Sublime for years and never had the issue of thinking there was an extra indent.

chrisdias commented 8 years ago

Why do you think that is? What is different between Sublime and VSCode given option 4, with perhaps some tweaks? i would love to nail that down so that we can perhaps have a lighter weight experience in Code.

jeffmcaffer commented 8 years ago

@Tyriar I used the new version of VS Code for seconds and had the problem :-) It could well be like a new pair of glasses (its my day for analogies). Everything looks warped but after a week or two its all ok. Then again, is it good that people need to "get used to" their tools?
As for sublime, I've not been using it so can't compare. Perhaps people who are used to Sublime don't have the problem. For me the problem was completely addressed by @anseki's 0 column ruler suggestion.

bgashler1 commented 8 years ago

@jeffmcaffer: what if we introduced a left-edge guide as very simple opt-in setting:

{
    // Show the left-edge of the code editor
    "editor.leftEdgeGuide":true;
}

Would this alleviate your concerns?

I'm concerned about adding this as a default for everyone, because it appears visually noisy for people who have never had a problem without it. Additionally, we could justify adding the Option 4 approach in by default.

jeffmcaffer commented 8 years ago

Would that be the explicit version of adding a 0 ruler? A bit redundant but better discoverability and reduces the pressure on choice of default (if people can easily find and change the setting). I'm agnostic on Option 4. It would not bother me but would not likely address the issue for me. Certainly would give it a try if it was there.

mattacosta commented 8 years ago

I am highly skeptical of option 4. Its effectiveness would seem to decrease with the number of indented lines currently being shown.

The core problem seems to be users not being able to tell where their whitespace is. It's solved in Sublime by its vertical indent guides (which is also visible in the screenshot above), and not by highlighting the line number. In my opinion, the best solutions for the issue are:

  1. Indent guides
    • Pros: Works no matter the number of indents.
    • Cons: Probably can't be implemented this close to release, also somewhat clutters the text area but could be turned on/off via options. Which may actually end up being a pro if this isn't a problem for those using 4-space indents, since they would see no change.
  2. Use a different gutter background color
    • Pros: Relatively simple to implement, better than a hard border (see below).
    • Cons: Only indicates where whitespace for the first column starts.
  3. VS-style folding lines
    • Pros: If you're going to put something in that's effectively like a border, it might as well also solve the "how much is this button going to collapse my text" question.
    • Cons: Only indicates where whitespace for the first column starts.
  4. Add a border to the gutter
    • Pros: Relatively simple to implement.
    • Cons: It's a border (see below).
  5. Set editor.renderWhitespace to true.
    • Just kidding. No one shows this. It does solve the core problem without a border though!

* This assumes a strategy of "no color change or borders > color change > border" in order to limit the creation of perceived UI sections.

anseki commented 8 years ago

In my case:

I'm using Sublime few years. I felt dissatisfied with undistinguishable indent. Then, I use draw_indent_guides and draw_white_space.

s01

"2 spaces indent" also might be the cause of this issue...

I often look at another part of code without changing current line. Therefore I hope that the edge of code area (i.e. all lines) is indicated. I think that it is solved by just a little changing a background color of gutter. (no border line)

anseki commented 8 years ago

When I am writing a line, of course current line is here. But I know indent at here before the current line marker indicates it, because I made that indent just now. When I am reading a code that was written, I may lose indent, and current line may not be here.

bgashler1 commented 8 years ago

Thanks for everyone's feedback! It really helped us understand the underlying concerns.

We will be addressing this issue most likely in our April release by introducing our own flavor of draw_indent_guides, which will remove the confusion about where an indent is, no matter what tab space size you use.

Since we won't have time to get it in by March, a workaround is to use @anseki's suggestion of having "editor.rulers": [0]. This will give you a left-hand reference. If you need more granularity, it may be helpful to also use "editor.renderWhitespace": true.

kuntau commented 8 years ago

Hi guys. Any idea which milestone this is on? I feel kind of silly backspacing over and over on the gutter edge thinking I might have accidental indent 😏

bgashler1 commented 8 years ago

@kuntau you can use a code ruler set at the far left zero position of the editable area. Add this snippet to your user settings "editor.rulers": [0]

Also, you can turn on indent guides to help further by adding this to your user settings "editor.renderIndentGuides": true

Hopefully these two things will help you out.

kuntau commented 8 years ago

@bgashler1 that's what I did after reading this issues. That is kind of hackey solution, I'm looking for official solution as discussed above, just a bit contrast between the gutter and editor area as suggestion #3 would be nice

alexdima commented 8 years ago

@kuntau Today there are the two ways described by @bgashler1 . I'm also looking into #14273 which will introduce a third way to workaround. We plan to make renderIndentGuides true out-of-the-box, but there is still a perf issue I wanna look into before doing that (some lookup in a tree is O(N) instead of O(H*log(N)).