Add `--keep-indentation` option by josephfrazier · Pull Request #1577 · prettier/prettier

josephfrazier · 2017-05-10T16:08:53Z

When this option is enabled, prettier will get the indentation of the
first line, determine its width (taking tabWidth into account),
subtract that width from printWidth, format the input with the new
printWidth, then add the first line's indentation back to every line.

Fixes #1324

When this option is enabled, prettier will get the indentation of the first line, determine it's width (taking `tabWidth` into account), subtracts that width from `printWidth`, format the input with the new `printWidth`, then add the first line's indentation back to every line. Fixes prettier#1324

vjeux · 2017-05-10T17:07:24Z

Thanks for making a pull request for this but I don't think that this should be part of prettier core. The use case for this is to format JavaScript code embedded inside of some other context. So the call site knows the indentation of the <script> tag and should indent the resulting code accordingly.

josephfrazier · 2017-05-10T17:11:49Z

Hmm, this wasn't really intended to address #1567 so much as #1324 (the more general case, without <script> tags). Did you change your mind from earlier at #1324 (comment)?

Yeah, this is something we should implement at some point. I honestly thought that it would have been needed for adoption but it turns out that people are going all in and not only converting entire files but also entire projects using it. This is a better outcome as it makes prettier a lot more valuable. But we still should support reformatting a range.

vjeux · 2017-05-10T17:15:22Z

What I meant by that is having the ability to pass prettier a range (or even a cursor position) within the file and reformat it. We want it to be smart enough to find the nearest Statement and only reformat this statement and have the correct indentation.

josephfrazier · 2017-05-10T17:22:24Z

Ohhh, I see now. Do you not think this is nevertheless useful as-is? I've already found it handy while working on prettier itself, to be able to make sure that my changes are prettier-formatted, without having to format the entire file (which currently causes large diffs).

vjeux · 2017-05-10T17:26:05Z

I do think that it is useful and want to support it, but it should be done correctly, just adding a flag --keep-indentation is not the right API. We should add a way to pass a range within that file and tweak prettier to reprint only this part of the ast.

josephfrazier · 2017-05-10T17:35:49Z

That's fair :) I'm a bit biased towards my approach here, as it's easier to integrate with my editor (Vim). However, if a more robust solution is implemented, I imagine Neoformat will be able to incorporate it.

vjeux · 2017-05-10T18:06:15Z

I'm not familiar with neoformat, what is the API that it expects for formatting a piece of code?

vjeux · 2017-05-10T18:07:41Z

This is the API I had in mind:
https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#textDocument_rangeFormatting

josephfrazier · 2017-05-10T18:13:54Z

Neoformat uses the command-line interface prettier --stdin, and pipes in the currently selected text, or the entire file if there is no selection. So, I'm hoping that the eventual implementation of range formatting will be available to the CLI (presumably, Neoformat would be extended to find and pass in the current range).

vjeux · 2017-05-10T19:01:38Z

So, every single code formatter supports a --keep-indentation option? That doesn't seem right.

josephfrazier · 2017-05-10T20:34:11Z

I don't understand what you're saying here. Can you clarify?

EDIT (didn't want to double-post): After using this to format selections in Vim a little more, I'm starting to see some limitations. Most glaringly, the selected text must parse as a valid JS program. For example, I can't format part of a function body that includes a return statement.

Ported from prettier/prettier#1577

lydell · 2017-05-10T21:35:55Z

Note that it is not safe for a text editor plugin to simply add a bunch of spaces at the beginning of each line because of multiline strings and template literals.

josephfrazier · 2017-05-10T21:40:46Z

That's a great point. Yeah... this feature as it's currently implemented here would definitely mangle multiline literals :( I hadn't considered that until you mentioned it, but I agree that it shouldn't be merged as-is.

See prettier/prettier#1577 (comment)

… the input These options default to `0` and `Infinity`, respectively, so that the entire input is formatted by default. However, if either option is specified such that a node lies completely outside the resulting range, the node will be treated as if it has a `// prettier-ignore` comment. Related to prettier#1577 (comment) Related to prettier#1324 Related to prettier#593

… the input (#1609) * Add `--range-start` and `--range-end` options to format only parts of the input These options default to `0` and `Infinity`, respectively, so that the entire input is formatted by default. However, if either option is specified such that a node lies completely outside the resulting range, the node will be treated as if it has a `// prettier-ignore` comment. Related to #1577 (comment) Related to #1324 Related to #593 * printer: Extract hasPrettierIgnoreComment() helper * Move isOutsideRange() to util * Don't throw errors about comments outside range "not printing" * Remove unnecessary check from isOutsideRange() * Make --range-end exclusive This lets it use the conventional way of specifying ranges in strings. Note that if the rangeEnd in the tests is changed to 158, it will fail, but it wouldn't have failed before this change. * Change range formatting approach NOTE: This doesn't pass its test yet. Note that since we're reading the indentation from the first line, it is expected not to change. However, a semicolon is added, and the lines outside the range are not changed. The new approach is roughly: * Require that the range exactly covers an integer number of lines of the input * Detect the indentation of the line the range starts on * Format the range's substring using `printAstToDoc` * Add enough `indent`s to the doc to restore the detected indentation * Format the doc to a string with `printDocToString` * Prepend/append the original input before/after the range See #1609 (comment) --- Given `tests/range/range.js`, run the following: prettier tests/range/range.js --range-start 165 --range-end 246 See the range's text with: dd if=tests/range/range.js ibs=1 skip=165 count=81 2>/dev/null * Don't use default function parameters Node v4 doesn't support them. See http://node.green/#ES2015-syntax-default-function-parameters * Hackily fix indentation of range formatting See #1609 (comment) Also update the snapshot to reflect that the indentation actually should decrease by one space, since there were 13 spaces in the input and we round down after dividing by tabWidth. * Revert "printer: Extract hasPrettierIgnoreComment() helper" See #1609 (comment) This reverts commit 62bf068. * Test automatically using the beginning of the rangeStart line and same for the end See #1609 (comment) * Fix automatically using the beginning of the rangeStart line and same for the end See #1609 (comment) * Propagate breaks after adding an indentation-triggering hardline See https://github.com/prettier/prettier/pull/1609/files/c1a61ebde8be73414c0a54bde3f323ac24295715#r116805581 * Extract getAlignmentSize(), use instead of countIndents() See https://github.com/prettier/prettier/pull/1609/files/c1a61ebde8be73414c0a54bde3f323ac24295715#r116804694 * Extract addAlignmentToDoc(), use instead of addIndentsToDoc() See https://github.com/prettier/prettier/pull/1609/files/c1a61ebde8be73414c0a54bde3f323ac24295715#r116804694 * Document that --range-start and --range-end include the entire line * Fix rangeStart calculation Before, it was incorrectly resulting in 1 when the originally provided value was 0 * Extract formatRange() helper function * Move getAlignmentSize() from printer to util This addresses #1609 (comment) * Move addAlignmentToDoc() from printer to doc-builders This addresses #1609 (comment)

josephfrazier added 2 commits May 10, 2017 12:05

Don't indent after last newline with --keep-indentation

1734e75

vjeux closed this May 10, 2017

josephfrazier added a commit to josephfrazier/prettier_d that referenced this pull request May 10, 2017

Add --keep-indentation option

5cf5db3

Ported from prettier/prettier#1577

josephfrazier added a commit to josephfrazier/prettier_d that referenced this pull request May 11, 2017

Warn that --keep-indentation mangles multiline literals

324bbdd

See prettier/prettier#1577 (comment)

josephfrazier mentioned this pull request May 15, 2017

Add --range-start and --range-end options to format only parts of the input #1609

Merged

josephfrazier mentioned this pull request Jun 19, 2018

Does not use printWidth from package.json with --keep-indentation josephfrazier/prettier_d#8

Closed

lock Bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 20, 2019

lock Bot locked as resolved and limited conversation to collaborators Jan 20, 2019

Uh oh!

Conversation

josephfrazier commented May 10, 2017

Uh oh!

vjeux commented May 10, 2017

Uh oh!

josephfrazier commented May 10, 2017

Uh oh!

vjeux commented May 10, 2017

Uh oh!

josephfrazier commented May 10, 2017

Uh oh!

vjeux commented May 10, 2017

Uh oh!

josephfrazier commented May 10, 2017

Uh oh!

vjeux commented May 10, 2017

Uh oh!

vjeux commented May 10, 2017

Uh oh!

josephfrazier commented May 10, 2017

Uh oh!

vjeux commented May 10, 2017

Uh oh!

josephfrazier commented May 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lydell commented May 10, 2017

Uh oh!

josephfrazier commented May 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

josephfrazier commented May 10, 2017 •

edited

Loading