New lines always create white spaces #3100

Closed
MiguelCastillo opened this Issue Mar 11, 2013 · 33 comments

Projects

None yet

6 participants

@MiguelCastillo
Member

When pressing enter to create a new line, white spaces are always inserted. For example, if you put the cursor at the beginning of a line and press enter, white spaces are inserted. This is really annoying if you use tabs.

This seems to be a recent regression.

Thanks,
Miguel

@TomMalbran
Contributor

Change the Tab settings at the status bar to use Tabs instead of Spaces and it should insert tabs when indenting instead of spaces.

@MiguelCastillo
Member

Hi, thank you for your response. I made sure I tried that before logging this issue. With either tabs or spaces selected in the status bar spaces are always inserted on new lines. I am running on sprint 21 with latest github changes. Did you have a second to try to reproduce the issue?

@njx njx was assigned Mar 18, 2013
@pthiess
Member
pthiess commented Mar 18, 2013

reviewed @njx

@njx
Member
njx commented Mar 19, 2013

@MiguelCastillo - I can't reproduce this on Mac in master or in sprint 21. If I create a blank JS file, switch the status bar item to Tabs, and then type function foo() { followed by Return, I get a hard tab (verified in hexdump). Are you on Mac or Win? Can you provide specific repro steps? Thanks.

@redmunds
Contributor

I can't reproduce on Win7 using same steps as NJ. I verified using the Brackets Show Whitespace extension (https://github.com/DennisKehrig/brackets-show-whitespace).

@MiguelCastillo
Member

Sorry I haven't gotten back to this. It is rather easy to reproduce. I am on win7 x64...

@njx I followed your steps and I can reproduce the issue with no problems. I am attaching a screenshot below... Not sure if the screenshot can show the issue clearly, but I left the cursor right between the two spaces that were inserted. I also created a temp repo to access the Untitles.js file I created to show the issue in the screenshot. I have a swf that shows me reproducing the issue, so if you want to watch me type it let me know :)

  1. Create new js file.
  2. Type function(){
  3. Press Enter
  4. The cursor goes the next line and two spaces are inserted automatically.
  5. In the new line, type function(){
  6. Press enter. Now the new line has a tab!

The issue happens with any file...
I have had this issue for a while now... I am running into this issue with latest source from github and with installations. I am generally running with latest brackets code and latest codemirror source (FYI. they have removed the sql mode in the 3.1.1, which causes Brackets to fail to load).

https://github.com/MiguelCastillo/bracket-whitespace/blob/master/Untitled.js

image

@MiguelCastillo
Member

I found what you need to do to reproduce the issue! Set you "Spaces" to a value other than "Tab Size". If your tab sizing is 4, change spaces to to 2 or 3 or whatever that isn't multiple of whatever tab sizing is...

I am sooo much happier now that I know how to get around the issue! Set white spaces and tabs to 4 :)

@redmunds
Contributor

@MiguelCastillo I am still not seeing the problem even with your new recipe. I am on Win7. Which version of Windows are you using.

Note that the CodeMirror SHA has been updated in Brackets, so use git submodule update to fix the sql mode problem. Does that fix this issue for you?

If not, can you install the Show Whitespace (https://github.com/DennisKehrig/brackets-show-whitespace) extension to help us all visualize the problem? Use Show > View Whitespace to enable it.

@redmunds
Contributor

I just noticed in the status bar of your screen shot it says "Tab Size 4". Maybe that's not clear, but it indicates to use Tabs instead of spaces. Click on "Tab Size" once and it will change to "Spaces". Does that fix it?

@MiguelCastillo
Member

AH!! Yes, that fixed it too. The usability for that setting is rather confusing. If the ui is displaying "Tab Size 4", I am expecting that tabs are being used for spaces. Switching to "Spaces 4" should give me spaces.

Hummm. The interesting thing is that when "Tab Spaces" and "Spaces" have the same value, they both insert tabs... I am not sure if this is an issue or just more confusion on my part :)

My question would then be what is the purpose of the little confusing setting?

Thanks for you help

@njx
Member
njx commented Mar 21, 2013

Also @MiguelCastillo - do you have any extensions installed? We've noticed that some extensions seem to mess with autoindent, e.g. Emmet.

@MiguelCastillo
Member

I have tried plain install of brackets with no extensions when providing feedback on the issue... The behavior is the same with and without the extensions I use, so the screenshots I have provided do show extensions being used. Here is a screenshot if my extensions' folder

image

@MiguelCastillo
Member

@redmunds I ended up merging the sql addon manually while things get sorted out, so I am all set with the newest codemirror for now :)

@redmunds
Contributor

Brackets uses "soft tabs", so if you have your preference set to insert 4 spaces, using the keyboard left/right arrows will advance by 4 spaces at a time, the same as if you had tabs. Maybe that's what you're seeing.

The Display Whitespace extension will show you exactly what you have (but it can make the editor a little sluggish, so I disable it when not using it).

According to your screenshot, all of the extensions are installed in Brackets. Move them to the Roaming/Brackets/extensions/disabled folder to disable them.

@MiguelCastillo
Member

As I mentioned before, I made sure to test without any extensions. Also tests with my extensions give the exact same result as without... The screenshot was just to show @njx that I am not using emmet whenever I have extensions active.

But yes @redmunds what you described is what I am seeing... That settings is rather confusing though. When I have "Spaces 4" on, I expect to get 4 spaces when pressing the return key and arrow keys will skip only 1 space at a time. And if I have "Tab Spaces 4" on I expect to get tabs when pressing the return key that are 4 spaces long and when pressing arrow keys, the cursor will jump 4 spaces at a time (a tab).

The setting gets even more confusing when spaces and tab spaces have different values. Not at all what I expected. As far as this issue is concerned, it seems like things are working as designed; whether good or bad usability.

Thanks for your feedback guys! :)
Miguel

@njx
Member
njx commented Mar 21, 2013

Hi @MiguelCastillo - you're exactly right. Either the behavior of the settings have changed, or they were always confusing and we just didn't test this case thoroughly enough.

I believe the behavior we originally intended is that the "Tab Size/Spaces" control is independent of the number control, and switching between "Tab Size" and "Spaces" is supposed to just switch whether you're using hard or soft tabs. The intended behavior should be (if the number is set to "n"):

  • If "Tab Size" is showing, then we insert hard tabs, hard tabs are set to be n spaces in width, and the default autoindent unit is one hard tab (which looks like n spaces).
  • If "Spaces" is showing, then we insert spaces, we have "soft tab" behavior that skips over every n spaces, and the default autoindent unit is also n spaces.

That's clearly not what's happening now: if you set a value while "Spaces" is showing, and then switch to "Tab Size", the number also changes. I think that's just wrong.

I'll try to see when this broke--it's possible that it broke when we switched to CodeMirror v3; maybe the semantics of the indent-related options changed and we didn't notice.

Thanks for sticking with us and patiently explaining the problem!

@TomMalbran
Contributor

The Tab Size and the Space Indentation numbers are different values. So you could have Tab Size and 4 and Spaces and 2, since there are 3 different values set in the Editor (Spaces/Tabs, Tab Size and Spaces Indentation). I believe that is the correct behavior here.

@redmunds
Contributor

I think this is working as designed. You can have different values for Spaces and Tabs.

Consider the case where you have Spaces 2 and Tab Size 4. Clicking on Spaces/Tab Size toggles between the settings.

When Spaces is showing, you'll get 2 spaces for each indent or Tab key. Brackets will not insert any Tab chars, but if you happen to have any in your page, they will be displayed a 4 spaces (which is why it says "Tab Size" instead of "Tabs"). And you get the soft tab behavior with spaces.

When you switch to Tab Size, then it's as you describe above. Brackets inserts a single Tab char and displays it to 4 spaces. You do not get soft tab behavior. The Spaces setting is not used, but it's remembered for when you switch back to Spaces.

@MiguelCastillo
Member

It makes sense to have two independent values if you want to extra flexibility, I actually do like it. But consider the size of this thread to explain something as simple as tab/space... There is something inherently wrong with the usability.

LOL, the setting isn't that big of a deal. I am just happy I understand how to get my tabs inserted when pressing the return key :)

@njx
Member
njx commented Mar 21, 2013

@redmunds - The problem occurs in the opposite case. If I set Spaces to 4 and Tabs to 2, then auto-indent indents 4 apparent spaces (2 hard tabs with 2-space widths), because of the hidden "4" setting on Spaces, which is not what I would expect--I wouldn't expect the hidden value to affect my current behavior.

I agree that you want to set all these things individually potentially, but I think the current design ends up conflating things in a confusing way.

I'm going to go ahead and call this "move to backlog" to discuss refining the UI here. Thanks everyone for your input!

@njx
Member
njx commented Mar 21, 2013

Marking "move to backlog" and removing from Sprint 22.

@TomMalbran
Contributor

I see the problem now. It seems like we are using indentUnit in the wrong way, since it should apply for both tabs and spaces and not just spaces as we do. I have a simple fix for this that I could post soon.

@redmunds
Contributor

@TomMalbran Brackets always inserts 1 tab char. The Tab Size setting is to tell the editor how to display the tab char. So indentUnit is always 1 with tabs. I think this is behaving as expected.

@TomMalbran
Contributor

If you have indentUnit set to 4 and Tab Size set to 2, then it inserts 2 tabs instead of 1. If you change the indentUnit option in CodeMirror to 2, then it will insert 1 tab as expected. It seems like CodeMirror uses the indentUnit for both tabs and spaces when indenting and not just spaces as how we handle it, so we need to change the indentUnit when changing the tabs.

@njx
Member
njx commented Mar 21, 2013

@TomMalbran - yup, exactly. I think your description is the way it used to work (at least, it's what I would have expected to happen :)). I'll see if I can git bisect to find out whether the behavior actually changed at some point.

@njx
Member
njx commented Mar 21, 2013

Nope, it looks like it's always worked this way, so it's not a regression. Leaving it as "move to backlog" so we can figure out what we want to do with it.

@TomMalbran
Contributor

I think it always worked like this. Check a look at my possible fix #3209.

@redmunds
Contributor

The problem occurs in the opposite case. If I set Spaces to 4 and Tabs to 2...

Yes, I'm seeing that problem.

@jasonsanjose
Member

Just catching up with this discussion. If it's helpful, the original pull request for this issue was #1801.

@redmunds redmunds closed this in 051f2da Mar 22, 2013
@redmunds redmunds reopened this Mar 22, 2013
@redmunds
Contributor

This was auto-closed, so re-opening. This now behaves as it was designed, but we may want to change the design.

Note that this (some what cryptic?) design was reached because Brackets does not have a Preferences dialog. Once a Preferences dialog is added, the UI can be more verbose about these settings, and the status bar can be simplified.

@MiguelCastillo
Member

I have pulled down these changes and when tab size is on, I get tabs inserted. This is what I was expecting.
When spaces are on, then spaces are inserted. This is what I expected.

So, the behavior is consistent, which is fantastic. 👍

I think that the cryptic part is due to trying to merge the setting for the number of spaces/tabs when pressing the return key and the number of units that are jumped over when pressing on the left/right arrows. "Treat spaces as soft tabs" type of setting is what would probably make this more clear. Maybe "Soft tabs: true | false" would suffice.

The usability of skipping N spaces to make the navigation feel like you are using tabs is great! It just isn't clear that such behavior is what's actually happening (soft tabs vs hard tabs).

@njx
Member
njx commented Mar 22, 2013

Thanks everyone for working on this. I think the behavior after @TomMalbran's fix makes sense (though it still seems a little odd to me that we retain different values for spaces vs. tabs), so it doesn't seem as urgent to change it. But it makes sense to me that people might not understand the soft tab behavior since there's no real indication in the UI that we do that (and they might want to turn it off). So I think we still want to potentially add a backlog item to let people control that independently. Leaving open as move to backlog for now.

/cc @GarthDB

@njx
Member
njx commented May 24, 2013

Captured in the backlog as https://trello.com/c/BwOd76dZ. Closing.

@njx njx closed this May 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment