This repository was archived by the owner on Mar 3, 2023. It is now read-only.
Fixed the indentation logic #10367
Closed
Fixed the indentation logic #10367
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the logic lacked handling of multiple matches of the
increase/decrease regexs, so things like the following javascript
would not be handled correctly (the last line would be indented).
if (test) {
foo({a:1
})};
var t = "this should be at indentation 0";
Fixed that.
Also revisited the logic regarding decreaseIndent and
decreaseNextIndent. It did not seem very logical that decreaseIndent
did *not* correspond to increaseIndent: decreaseIndent regarded the
current line whereas increaseIndent regarded the next line (just like
decreaseNextIndent). For that reason I renamed decreaseIndent to
decreaseThisIndent and decreaseNextIndent to decreaseIndent. This way
we also avoid a whole lot of issues related to the concept of
decreaseNextIndent, because most of the language packages I looked at
have not yet adopted it but the semantics of decreaseIndent had
changed. So now the semantics of decreaseIndent is back to what it was
and decreaseThisIndent is the new concept.
Previously the logic lacked handling of multiple matches of the
increase/decrease regexs, so things like the following javascript
would not be handled correctly (the last line would be indented).
if (test) {
foo({a:1
})};
var t = "this should be at indentation 0";
Fixed that.
Also revisited the logic regarding decreaseIndent and
decreaseNextIndent. It did not seem very logical that decreaseIndent
did *not* correspond to increaseIndent: decreaseIndent regarded the
current line whereas increaseIndent regarded the next line (just like
decreaseNextIndent). For that reason I renamed decreaseIndent to
decreaseThisIndent and decreaseNextIndent to decreaseIndent. This way
we also avoid a whole lot of issues related to the concept of
decreaseNextIndent, because most of the language packages I looked at
have not yet adopted it but the semantics of decreaseIndent had
changed. So now the semantics of decreaseIndent is back to what it was
and decreaseThisIndent is the new concept.
Also now using the tokenized line to determine whether a in-/decrease
pattern appears within a quoted string (and ignoring it in that case),
rather than requiring regexes for checking whether or not the
occurance is within a quoted string or not -- the examples I saw for
that were wrong, too (e.g., things like /} [^"] / are non-sense,
because things like '} var x = "hey"' are perfectly fine). One really
needs to reason about balanced quotation to get it right, and the
tokenizer already does that for us, so let's use it rather than
reinventing the wheel. The decrease/increase regexes can now be much
simpler in the language packages.
This was referenced Jan 11, 2016
Contributor
Author
|
This new logic is best tried with the following PR for javascript: atom/language-javascript#301. |
|
I don't usually post |
Contributor
There was a problem hiding this comment.
Just saying, YAML also has string.unquoted, so you might want to adjust for that.
Contributor
Author
There was a problem hiding this comment.
Good point. I didn't know about that. However it seems that unquoted strings must not contain any special characters, so I'm not sure I can think of a language and example where this would be necessary. Can you think of a good test case (incl. language) we could add for that?
This way, if decreaseThisIndentPattern is not specified, we fall back to the previous name for that pattern (decreaseIndentPattern), while still being able to change the semantics for decreaseIndentPattern. This is because we never outdent more than one on the decreaseThisIndentPattern -- which is what the behavior used to be (and which has some problems when closing more than one scope).
Contributor
Author
|
Found a way to make the transition smoother. Will create a new PR. |
Contributor
Author
|
The new PR is #10384 (comment). |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the logic lacked handling of multiple matches of the
increase/decrease regexs, so things like the following javascript
would not be handled correctly (the last line would be indented).
Fixed that.
Also revisited the logic regarding decreaseIndent and
decreaseNextIndent. It did not seem very logical that decreaseIndent
did not correspond to increaseIndent: decreaseIndent regarded the
current line whereas increaseIndent regarded the next line (just like
decreaseNextIndent). For that reason I renamed decreaseIndent to
decreaseThisIndent and decreaseNextIndent to decreaseIndent. This way
we also avoid a whole lot of issues related to the concept of
decreaseNextIndent, because most of the language packages I looked at
have not yet adopted it but the semantics of decreaseIndent had
changed. So now the semantics of decreaseIndent is back to what it was
and decreaseThisIndent is the new concept.
Also now using the tokenized line to determine whether a in-/decrease
pattern appears within a quoted string (and ignoring it in that case),
rather than requiring regexes for checking whether or not the
occurrence is within a quoted string or not -- the examples I saw for
that were wrong, too (e.g., things like
/} [^"] /are non-sense,because things like
} var x = "hey"are perfectly fine). One reallyneeds to reason about balanced quotation to get it right, and the
tokenizer already does that for us, so let's use it rather than
reinventing the wheel. The decrease/increase regexes can now be much
simpler in the language packages.