8000 Print objects across lines only when needed by jlongster · Pull Request #347 · benjamn/recast · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Print objects across lines only when needed #347

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
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Print objects across lines only when needed #347

wants to merge 1 commit into from

Conversation

jlongster
Copy link
Contributor

This is the biggest change. This avoids breaking objects across lines in many cases.

Take the following code:

const { val1, val2 } = require("obj");

function x() {
  return { foo: 1, bar: 2 };
}

The previous behavior outputs:

const {
  val1,
  val2
} = require("obj");

function x() {
  return {
    foo: 1,
    bar: 2
  };
}

This PR outputs:

const { val1, val2 } = require("obj");

function x() {
  return { foo: 1, bar: 2 };
}

The goal is to only break objects when it needs to, which is when one of two things happen:

  • The width of the object is more than the options.wrapColumn threshold
  • Any of the values span multiple lines

If neither of these are true, it's safe to keep it on the same line, which I think cleans up a lot of code that uses simple objects.

I haven't worked on the tests yet but if anyone can verify this I can fix up the tests.

@benjamn
Copy link
Owner
benjamn commented Dec 1, 2016

I think those two criteria for splitting make sense, and working on the tests would be a worthwhile next step.

Some little things have changed, like printing { } for empty objects intead of {} when options.objectCurlySpacing is true (I prefer the {}, as before).

Some of this code appears to be cleanup/refactoring of the original logic, and some of it implements your changes. Perhaps split the changes into two commits, for clarity?

Also, what about array literals?

@jlongster
Copy link
Contributor Author

Also, what about array literals?

Good point, I haven't looked at those at all. Similar heuristics are probably appropriate there.

I'll try to find time to wrap up my PRs; I have a work week next week so this weekend I'll be getting ready for that and then busy all next week. So worst case I might not update them, but don't think I've forgotten about it. I'll probably be able to work on them though.

I can try to split up the refactoring into two commits, but it might be a bit hard. The refactoring only made sense with my new logic so I don't know how hard it will be re-implement it in two steps. I can try.
873A

@keyz keyz mentioned this pull request Jan 13, 2017
Open
@manuel3108 manuel3108 mentioned this pull request Jul 27, 2024
Closed
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.
2 participants
@jlongster @benjamn
0