Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Auto-Identation and Switch Statements #1324

Closed
jdiehl opened this issue Aug 1, 2012 · 15 comments
Closed

Auto-Identation and Switch Statements #1324

jdiehl opened this issue Aug 1, 2012 · 15 comments

Comments

@jdiehl
Copy link
Contributor

jdiehl commented Aug 1, 2012

I noticed that the auto-indentation seems to be confused after switch statements. Consider the following (JSLint-approved) code:

function foo(a) {
    "use strict";
    switch (a) {
    case 1:
        break;
    }
}

When hitting enter after the first closing }, auto-indentation inserts 3 tabs (1 would be correct).
When hitting enter after the second closing }, auto-indentation again inserts 3 tabs (0 would be correct).
This goes on, even if additional functions, scopes, whatever are defined, auto-indentation always adds 3 tabs.

@ghost ghost assigned pthiess Aug 7, 2012
@pthiess
Copy link
Contributor

pthiess commented Aug 7, 2012

Reviewed - Move to backlog - card 379.

@peterflynn
Copy link
Member

I get 2 tabs after both closing curly braces instead of 3, if this is the only text in the file.

This is a CodeMirror bug that repros outside of Brackets as well. I've filed codemirror/codemirror5#708 to track it.

@peterflynn
Copy link
Member

Looks like the CodeMirror bug has been fixed. It might be a while before we can merge though, since there's been a lot of churn in the scrolling code lately. @jdiehl, do you want to verify the fix upstream in the meantime?

@jdiehl
Copy link
Contributor Author

jdiehl commented Aug 22, 2012

This works on CodeMirror.net, so I guess it is fixed. Thanks for forwarding this.

@jdiehl jdiehl closed this as completed Aug 22, 2012
@peterflynn
Copy link
Member

Reopening -- this won't be fixed in Brackets until we integrate a newer version of CodeMirror

@pthiess
Copy link
Contributor

pthiess commented Jan 11, 2013

Marked low priority

@njx
Copy link
Contributor

njx commented Feb 13, 2013

The worst of this appears to be fixed now that cmv3 is merged--the indentation doesn't "march inward"--but the indentation level of the case statement is too high (it gets indented two tabs worth instead of one, at least when spaces are on). Assigning to me

@ghost ghost assigned njx Feb 13, 2013
@njx
Copy link
Contributor

njx commented Jun 12, 2013

Filed codemirror/codemirror5#1592 on the latter issue.

@fdecampredon
Copy link

This is due to the overriding of electricChars on the Editor.
CodeMirror javascript mode double indent every line in a switch statement except the one with case and default at the beginning.
The problem is that in JavaScript Mode the ":" is an electric char (which is not the case of the _checkElectricChars hardcoded RegExp), so in CodeMirror when you type ':' the indentation on case or default lines are updated, what does not happen in brackets.
I guess a simple fix would be to include the ":" in the _checkElectricChars methods. (in fact does this _checkElectricChars methods could be removed ?)

@njx njx added this to the Brackets 1.0 milestone Mar 14, 2014
@njx
Copy link
Contributor

njx commented Mar 14, 2014

Low pri but nominating for Brackets 1.0 - it looks like a fix should be possible or easy now (an option was added to CM, and also there's @fdecampredon's note).

@njx
Copy link
Contributor

njx commented Apr 11, 2014

Reviewed. Timebox to half hour.

@njx njx added the F Editor label Jul 12, 2014
@njx njx removed their assignment Jul 12, 2014
@pthiess pthiess removed the tracking label Aug 15, 2014
@peterflynn peterflynn self-assigned this Aug 15, 2014
@peterflynn
Copy link
Member

Unfortunately the CM flag added in codemirror/codemirror5#1592 doesn't really help the issue -- it just makes CM insist on intending switch statements completely flat, so things under case aren't indented another stop. And if you manually hit tab on the line under a case, CM will yank the indent level back one every time you hit Enter :-(

It sounds like the real fix is to have _checkElectricChars() trigger on :, but there are several wrinkles there:

  • We probably don't want it in languages other than JS (e.g., typing ":" in the middle of HTML text causing indent to shift around would be weird).
  • We may not want it in non-switch contexts even in JS (e.g. typing an object literal). This seems like the hardest bit to solve, so hopefully it doesn't turn out to be an issue.
  • The current _checkElectricChars() exits early if the electric char isn't the first non-whitespace char on the line. We can't do that in this case because the switch <value> part comes before the :.

@peterflynn
Copy link
Member

It also still seems like a bug in CM that the initial line after switch () { is double-indented though, because there's basically no situation where you'd put stuff before the first case statement (and if you did, e.g. some top-level comment, you probably wouldn't want it double-indented either).

@dangoor dangoor removed this from the Release 1.1 milestone Dec 8, 2014
@dangoor
Copy link
Contributor

dangoor commented Dec 8, 2014

Remove 1.1 milestone. We'll take another look at this when we review Marcel's pull request.

@prafulVaishnav
Copy link
Contributor

Fixed by PR #9387 Closing it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants