-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minimal indenting #286
Minimal indenting #286
Conversation
This looks like a great first stab. I agree with filing an issue to improve the algorithm later, it's more important to get the functionality shipped. Thanks! On Sun, Jun 9, 2013 at 9:09 AM, Liam Newman notifications@github.com
|
@@ -1020,6 +1027,11 @@ def test_beautifier(self): | |||
bt('if (foo) // comment\n (bar());'); | |||
bt('if (foo) // comment\n (bar());'); | |||
bt('if (foo) // comment\n /asdf/;'); | |||
bt('this.oa = new OAuth(,\n' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is leading comma intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, typo. Fixed.
Oh yeah, this doesn't address #256, but it could be adapted to cover that eventually. |
This is a functional initial implementation.
I could reverse this - instead of removing redundant, under indenting and then adding missing when the first newline is added. This would produce less reprocessing of the output in most cases, but that would also make the line wrapping issue noted in #284 even more pronounced. There are simpler ways to improve this such as representing the results as an array of Line objects for quicker access to each location where fix up is needed. Or enhancing that further to have indentation represented by a number instead of characters on each Line. Then modification would be simply changing a number instead of modifying an array.
@einars , @evocateur, unless you object or have feedback, in a couple of days I'll push this change and file an issue to revisit and improve this design later.
Closes #281.