Skip to content

Preserve blank lines#208

Merged
Constellation merged 5 commits intoestools:masterfrom
k4b7:preserveBlankLines
Jan 7, 2015
Merged

Preserve blank lines#208
Constellation merged 5 commits intoestools:masterfrom
k4b7:preserveBlankLines

Conversation

@k4b7
Copy link
Copy Markdown
Contributor

@k4b7 k4b7 commented Nov 2, 2014

I'm using escodegen to reformat user code in the Khan Academy live-editor. If a student adds spaces between lines to make things more reabable we'd like to maintain that format while fixing the indentation and other issues that might exist.

This submission fixes some issues from the previous one.

@review-ninja
Copy link
Copy Markdown

review-ninja commented Nov 2, 2014

ReviewNinja

@k4b7
Copy link
Copy Markdown
Contributor Author

k4b7 commented Nov 2, 2014

This option uses the original source code and the ranges/extendedRanges in the AST nodes to figure out where line breaks should be inject to maintain the spacing that was in the original source code.

@k4b7 k4b7 mentioned this pull request Nov 2, 2014
Closed
@Constellation
Copy link
Copy Markdown
Member

Constellation commented Dec 19, 2014

Oh, sorry for the late reply. I'll review it at weekend.

@k4b7
Copy link
Copy Markdown
Contributor Author

k4b7 commented Dec 19, 2014

Cool. I'll rebase the code so that it's ready to merge.

@k4b7 k4b7 force-pushed the preserveBlankLines branch from 3b7a364 to cfd5c61 Compare December 19, 2014 21:40
@k4b7
Copy link
Copy Markdown
Contributor Author

k4b7 commented Dec 19, 2014

I tried rebasing but I had branched before the giant switch statement was replaced with the CodeGenerator.Expression dictionary so doing a merge is probably more complicated than just porting the changes by hand. I'll try to get it done this evening weekend.

@k4b7 k4b7 force-pushed the preserveBlankLines branch from cfd5c61 to ee5d108 Compare December 22, 2014 03:30
@k4b7 k4b7 mentioned this pull request Dec 22, 2014
Closed
Constellation
Constellation reviewed Dec 22, 2014
View reviewed changes
Comment thread escodegen.js Outdated
Copy link
Copy Markdown
Member

@Constellation Constellation Dec 22, 2014

Choose a reason for hiding this comment

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

Could you combine this VariableDeclaration with the above one?

@k4b7
Copy link
Copy Markdown
Contributor Author

k4b7 commented Jan 7, 2015

@Constellation Let me know if there are any other changes you want me to make.

@Constellation
Copy link
Copy Markdown
Member

Constellation commented Jan 7, 2015

LGTM!

Constellation added a commit that referenced this pull request Jan 7, 2015
@Constellation
Merge pull request #208 from kevinb7/preserveBlankLines
9d6530a 
Preserve blank lines
@Constellation Constellation merged commit 9d6530a into estools:master Jan 7, 2015
@Constellation
Copy link
Copy Markdown
Member

Constellation commented Jan 7, 2015

Thank you for your great contribution!

@Constellation
Copy link
Copy Markdown
Member

Constellation commented Jan 7, 2015

And sorry for further late reply.

@k4b7
Copy link
Copy Markdown
Contributor Author

k4b7 commented Jan 7, 2015

@Constellation No worries. Thanks for the merge!

@k4b7 k4b7 deleted the preserveBlankLines branch January 7, 2015 15:02
@k4b7 k4b7 mentioned this pull request Jan 7, 2015
Merged
@Constellation
Copy link
Copy Markdown
Member

Constellation commented Jan 7, 2015

Released it as 1.5.0 :D

@simondel simondel mentioned this pull request Dec 1, 2015
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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

@k4b7 @review-ninja @Constellation