Skip to content

Drop dedent-on-enter for "return" statements. - #6939

Merged
Eric Snow (ericsnowcurrently) merged 21 commits into
microsoft:masterfrom
ericsnowcurrently:drop-return-dedent
Aug 13, 2019
Merged

Drop dedent-on-enter for "return" statements.#6939
Eric Snow (ericsnowcurrently) merged 21 commits into
microsoft:masterfrom
ericsnowcurrently:drop-return-dedent

Conversation

@ericsnowcurrently

@ericsnowcurrently Eric Snow (ericsnowcurrently) commented Aug 12, 2019

Copy link
Copy Markdown

(for #6813)

We'll work on adding support back in #6564.

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Has a news entry file (remember to thank yourself!)
  • Appropriate comments and documentation strings in the code
  • Has sufficient logging.
  • [ ] Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated
  • [ ] Test plan is updated as appropriate
  • [ ] package-lock.json has been regenerated by running npm install (if dependencies have changed)
  • [ ] The wiki is updated with any design decisions/details.

@ericsnowcurrently
Revert "Revert "Add regex to dedent else and friends (microsoft#6497)…
b8ae859 
…" (microsoft#6945)"

This reverts commit 2402dd5.
@ericsnowcurrently
Copy IndentAction into the mock VSCode.
9b94f35
@ericsnowcurrently
Test against getLanguageConfiguration().
319dd87
@ericsnowcurrently
language config formatting
d1da292
@ericsnowcurrently
Inline the regexes.
9add75c
@ericsnowcurrently
Add a verboseRegExp() helper.
465147f
@ericsnowcurrently
Use verbose regexes in the language config.
e90bcbb
@ericsnowcurrently
Make the regex tests a little easier to read/modify.
09f4110
@ericsnowcurrently
Correct the break/continue/return regex.
cbbb419
@ericsnowcurrently
Fill in language config regex test coverage.
0ccf9cd
@ericsnowcurrently
Minor fixes to language config regex patterns.
02cee4b
@ericsnowcurrently
Drop support for indent-on-enter for "return *".
cc055fe
@ericsnowcurrently
Drop indent-on-enter for "return" completely.
c20ea1c
@ericsnowcurrently
Drop an erroneous test.
1ff16dc
@ericsnowcurrently
Add a NEWS entry.
b630586
@ericsnowcurrently
Add tests for verboseRegExp().
395442f
@ericsnowcurrently
Move indentationRules back to onEnterRules.
dea0c9a
@ericsnowcurrently
Move a comment.
ad4a4be
kimadeline
Kim-Adeline Miguel (kimadeline) reviewed Aug 12, 2019
View reviewed changes
Comment thread src/client/language/languageConfiguration.ts
kimadeline
Kim-Adeline Miguel (kimadeline) previously requested changes Aug 12, 2019
View reviewed changes

@kimadeline Kim-Adeline Miguel (kimadeline) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, only got a couple of questions about (╯°□°)╯︵ ┻━┻ sǝxǝƃǝɹ

Comment thread src/client/language/languageConfiguration.ts
Comment thread src/client/language/languageConfiguration.ts
Comment thread src/client/language/languageConfiguration.ts
DonJayamanne
Don Jayamanne (DonJayamanne) reviewed Aug 13, 2019
View reviewed changes
Comment thread src/test/language/languageConfiguration.unit.test.ts Outdated
DonJayamanne
Don Jayamanne (DonJayamanne) reviewed Aug 13, 2019
View reviewed changes
Comment thread src/client/common/utils/regexp.ts
DonJayamanne
Don Jayamanne (DonJayamanne) reviewed Aug 13, 2019
View reviewed changes
Comment thread src/test/common/utils/regexp.unit.test.ts Outdated
DonJayamanne
Don Jayamanne (DonJayamanne) reviewed Aug 13, 2019
View reviewed changes

@DonJayamanne Don Jayamanne (DonJayamanne) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ericsnowcurrently

Eric Snow (ericsnowcurrently) commented Aug 13, 2019

Copy link
Copy Markdown
Author

Thanks for the review, Kim-Adeline Miguel (@kimadeline). I'd like to talk with you about this PR a bit more, especially since it refactors a bunch of the code that you original wrote. I'll ping you.

Also, note that to make this PR work I un-reverted #6497 and then removed the indentationRules part right after that. I'm pretty sure I have it right, especially since the same tests pass, but please pay extra attention to make sure I didn't miss something and accidentally re-introduce the bad behavior we reverted. Thanks!

@ericsnowcurrently
Eric Snow (ericsnowcurrently) dismissed Kim-Adeline Miguel (kimadeline)’s stale review August 13, 2019 17:11

need to merge this and Don already reviewed (will still check with Kim)

kimadeline
Kim-Adeline Miguel (kimadeline) approved these changes Aug 13, 2019
View reviewed changes

@kimadeline Kim-Adeline Miguel (kimadeline) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you 👍

@ericsnowcurrently
Eric Snow (ericsnowcurrently) merged commit 11dd074 into microsoft:master Aug 13, 2019
@ericsnowcurrently
Eric Snow (ericsnowcurrently) deleted the drop-return-dedent branch August 13, 2019 20:05
@lock lock Bot locked as resolved and limited conversation to collaborators Aug 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DonJayamanne Don Jayamanne (DonJayamanne) DonJayamanne approved these changes
+1 more reviewer
@kimadeline Kim-Adeline Miguel (kimadeline) kimadeline approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ericsnowcurrently @DonJayamanne @kimadeline