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.

Sign up for GitHub

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

Jump to bottom

[MemberExpression] Long object access doesn't wrap #21

Closed
JakeCoxon opened this issue Jan 10, 2017 · 6 comments
Closed

[MemberExpression] Long object access doesn't wrap #21

JakeCoxon opened this issue Jan 10, 2017 · 6 comments
Labels
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

@JakeCoxon
Copy link

Not sure if this is a bug or just a rare enough case that it doesn't matter

Input

something()
  .something
  .something()
  .something
  .something()
  .something
  .something()

Output

something().something.something().something.something().something.something();

(Longer than 60 chars)

Input

someVeryLongMethodObjectName
  .withSomeVeryLongObjectKey
  .andAnotherLongKey;

Output

someVeryLongMethodObjectName.withSomeVeryLongObjectKey.andAnotherLongKey;
@vramana vramana added the type:bug Issues identifying ugly output, or a defect in the program label Jan 11, 2017
Copy link
Contributor

Repro case

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22someVeryLongMethodObjectName%5Cn%20%20.withSomeVeryLongObjectKey%5Cn%20%20.withSomeVeryLongObjectKey%5Cn%20%20.andAnotherLongKey%3B%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Afalse%7D%7D

@despairblue
Copy link
Collaborator

@JakeCoxon If you want to give it a shot you might be interested in the discussion in #212 and my take on this (which created a lot of other issues) in #217.

I'd still like to see a solution for this. Chai assertions are another thing that suffer from this.

I wonder if there is a way to only add an softspace after a dot if the currently printed line would exceed the max line length with the next token. But I think that's not easy at all and not performant.

Copy link
Contributor

@despairblue do you mind giving a real-world example of chai assertions that do not look good? It's usually better for this kind of things to look at real code rather than make up examples.

@despairblue
Copy link
Collaborator

despairblue commented Jan 25, 2017 •
edited
Loading

@vjeux sure this would be a small one. I'd rather see the left case instead of the right one.

Or this one which is a little longer.

Copy link
Contributor

@despairblue thank you! This is great! Shouldn't be too hard to make work.

Copy link
Contributor

With #462 it looks like

if (testConfig.ENABLE_ONLINE_TESTS === "true") {
  describe("POST /users/me/pet", function() {
    it("saves pet", function() {
      function assert(pet) {
        expect(pet).to.have.property("OwnerAddress").that.deep.equals({
          AddressLine1: "Alexanderstrasse",
          AddressLine2: "",
          PostalCode: "10999",
          Region: "Berlin",
          City: "Berlin",
          Country: "DE"
        });
      }
    });
  });
}

which is better. But we should also be able to break the .to.have.property()

Closed
Jan 25, 2017
Reimplement MemberExpression printing
b2d89d2 
There are currently three issues related to suboptimal rendering of MemberExpression chains. The previous implementation was trying to flatten only a single group at the same time, but it didn't work well because we didn't have the full context to be able to make decisions.

In this implementation, I'm going through the entire chain at the same time and group it into logical units and make decisions based on this. It solves all the problems I can think of and if we need to tweak it in the future, it should be easy.

Fixes prettier#268
Fixes prettier#212
Fixes prettier#21
Merged
Jan 28, 2017
Reimplement MemberExpression printing
59af35b 
There are currently three issues related to suboptimal rendering of MemberExpression chains. The previous implementation was trying to flatten only a single group at the same time, but it didn't work well because we didn't have the full context to be able to make decisions.

In this implementation, I'm going through the entire chain at the same time and group it into logical units and make decisions based on this. It solves all the problems I can think of and if we need to tweak it in the future, it should be easy.

Fixes prettier#268
Fixes prettier#212
Fixes prettier#21
Jan 28, 2017
Reimplement MemberExpression printing
ca151d9 
There are currently three issues related to suboptimal rendering of MemberExpression chains. The previous implementation was trying to flatten only a single group at the same time, but it didn't work well because we didn't have the full context to be able to make decisions.

In this implementation, I'm going through the entire chain at the same time and group it into logical units and make decisions based on this. It solves all the problems I can think of and if we need to tweak it in the future, it should be easy.

Fixes prettier#268
Fixes prettier#212
Fixes prettier#21
Jan 28, 2017
Reimplement MemberExpression printing
1dc7dd5 
There are currently three issues related to suboptimal rendering of MemberExpression chains. The previous implementation was trying to flatten only a single group at the same time, but it didn't work well because we didn't have the full context to be able to make decisions.

In this implementation, I'm going through the entire chain at the same time and group it into logical units and make decisions based on this. It solves all the problems I can think of and if we need to tweak it in the future, it should be easy.

Fixes prettier#268
Fixes prettier#212
Fixes prettier#21
Jan 31, 2017
Reimplement MemberExpression printing
5f7a379 
There are currently three issues related to suboptimal rendering of MemberExpression chains. The previous implementation was trying to flatten only a single group at the same time, but it didn't work well because we didn't have the full context to be able to make decisions.

In this implementation, I'm going through the entire chain at the same time and group it into logical units and make decisions based on this. It solves all the problems I can think of and if we need to tweak it in the future, it should be easy.

Fixes prettier#268
Fixes prettier#212
Fixes prettier#21
Jan 31, 2017
Reimplement MemberExpression printing (#469)
d0bc299 
There are currently three issues related to suboptimal rendering of MemberExpression chains. The previous implementation was trying to flatten only a single group at the same time, but it didn't work well because we didn't have the full context to be able to make decisions.

In this implementation, I'm going through the entire chain at the same time and group it into logical units and make decisions based on this. It solves all the problems I can think of and if we need to tweak it in the future, it should be easy.

Fixes #268
Fixes #212
Fixes #21
josephfrazier pushed a commit to josephfrazier/prettier that referenced this issue Jun 20, 2017
@langdonx @rhengles
breakBeforeElse fix (prettier#21)
a1f52d7
@azz azz mentioned this issue Aug 27, 2017
Merged
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 8, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
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
Milestone
No milestone
Development

No branches or pull requests
4 participants
@JakeCoxon @despairblue @vramana