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

Too much indentation in method body selected by range if first line is comment or incompletely selected #4926

Closed
cmcaine opened this issue Aug 1, 2018 · 7 comments · Fixed by #8410
Labels
area:ranges Issues about formatting/ignoring/etc segments of files lang:javascript Issues affecting JS locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program

Comments

@cmcaine
Copy link

cmcaine commented Aug 1, 2018

Prettier 1.14.0
Playground link

--parser babylon
--range-end 304
--range-start 101

Can reproduce with just range 100 - 101 (the newline character after the brace on line 4), so that seems to be the culprit.

If I replace line 4 with an expression (e.g. let x) then the method is formatted correctly more often so long as my selection covers the second character of the expression, but you can break it again by moving the start of the selection over the brace on line 3.

Input:

class Foo {
    /** Does this key match a given MinimalKey extending object? */
    match(keyevent) {
        // 'in' doesn't include prototypes, so it's safe for this object.
        for (let attr in this) {
            if (this[attr] !== keyevent[attr]) return false
        }
        return true
    }
}

Output:

class Foo {
    /** Does this key match a given MinimalKey extending object? */
    match(keyevent) {
                      // 'in' doesn't include prototypes, so it's safe for this object.
                      for (let attr in this) {
                        if (this[attr] !== keyevent[attr]) return false;
                      }
                      return true;
                    }
}

Expected behavior:

Don't indent so much. Don't be so sensitive to range choice.

Ref: nrwl/precise-commits#15

@ikatyang ikatyang added the type:bug Issues identifying ugly output, or a defect in the program label Aug 2, 2018
@j-f1
Copy link
Member

j-f1 commented Aug 3, 2018

I’m starting to wonder if we should change range formatting to format the whole file, but with two “cursors,” then replace what’s in the range with the text between the cursors.

@cmcaine
Copy link
Author

cmcaine commented Aug 3, 2018

Sounds sensible to me. In my project we just ended up running prettier on everything so that we could use whole file prettier.

@sheerun
Copy link
Contributor

sheerun commented Aug 28, 2019

This makes range formatting unusable. Prettier randomly indents half of codebase

@sheerun
Copy link
Contributor

sheerun commented Aug 28, 2019

btw. to make it worse range formatting is very slow because of API prettier is exposing (you can provide just one range, so if you need to format n ranges you need to call format n times)

@lydell
Copy link
Member

lydell commented Aug 28, 2019

@sheerun Kind of related: #5807 (comment)

@sheerun
Copy link
Contributor

sheerun commented Aug 28, 2019

Here's another even simpler example:

test.js

function test() {
  'foo';
  'bar';
}

command:

cat test.js | prettier --parser babel --range-start 25 --range-end 25
function test() {
                  ("foo");
                  ("bar");
                }

@thorn0 thorn0 added the area:ranges Issues about formatting/ignoring/etc segments of files label Nov 23, 2019
@thorn0
Copy link
Member

thorn0 commented Dec 2, 2019

See one more manifestation of this bug in #7082.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:ranges Issues about formatting/ignoring/etc segments of files lang:javascript Issues affecting JS locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants