Skip to content
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

Improve inline js formatting #109

Merged
merged 3 commits into from
Jan 3, 2019
Merged

Conversation

DylanPiercey
Copy link
Contributor

Description

This PR refactors the JS formatting portion of the code to use only one function "formatJS" which now will format either a string of code or ast nodes.

Some changes to the output include:

  • Prettier formatted JS code will now properly honor the current print depth.
  • If the output from prettier would have yielded a single line scriptlet the block is omitted.
  • tag and attribute arguments are now attempted to be formatted.

This PR also includes improvements to the concise mode attribute printing:

  • Previously multi line attributes had an additional indentation
  • Previously only the first line of multi line attributes was indented properly
  • Now omits the [] on single attributes that span multiple lines

Checklist:

  • I have updated/added documentation affected by my changes.
  • I have added tests to cover my changes.

@DylanPiercey DylanPiercey force-pushed the improve-inline-js-formatting branch from 443e75e to 0438bc8 Compare December 31, 2018 16:30
@DylanPiercey DylanPiercey force-pushed the improve-inline-js-formatting branch from 754cc29 to adcf091 Compare December 31, 2018 17:11
@codecov
Copy link

codecov bot commented Dec 31, 2018

Codecov Report

Merging #109 into master will increase coverage by 2.73%.
The diff coverage is 97.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #109      +/-   ##
==========================================
+ Coverage   40.21%   42.95%   +2.73%     
==========================================
  Files          65       66       +1     
  Lines        1840     1930      +90     
==========================================
+ Hits          740      829      +89     
- Misses       1100     1101       +1
Impacted Files Coverage Δ
packages/prettyprint/src/PrintContext.js 100% <100%> (ø) ⬆️
packages/prettyprint/src/prettyPrintAST.js 93.75% <100%> (+0.41%) ⬆️
packages/prettyprint/src/prettyPrintSource.js 89.47% <100%> (+0.58%) ⬆️
packages/prettyprint/src/util/toCode.js 100% <100%> (ø) ⬆️
packages/prettyprint/src/printHtmlElement.js 100% <100%> (ø) ⬆️
packages/migrate/src/index.js 91.3% <100%> (+0.39%) ⬆️
packages/prettyprint/src/util/formatJS.js 100% <100%> (ø) ⬆️
packages/prettyprint/src/printScriptlet.js 100% <100%> (ø) ⬆️
packages/prettyprint/src/util/getTextValue.js 100% <100%> (ø) ⬆️
packages/prettyprint/src/util/formatArgument.js 93.33% <93.33%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 622145b...adcf091. Read the comment docs.

@DylanPiercey DylanPiercey merged commit 2336c7e into master Jan 3, 2019
@mlrawlings mlrawlings deleted the improve-inline-js-formatting branch May 7, 2020 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants