Skip to content

Ignore cursorOffset in format() and check() #12803

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

Merged
merged 1 commit into from
May 8, 2022

Conversation

fisker
Copy link
Member

@fisker fisker commented May 7, 2022

Description

Maintain cursorOffset seems expensive to me, and it's useless in format() and check()

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

const { formatted } = await formatWithCursor(text, opts);
async function format(text, options) {
const { formatted } = await formatWithCursor(text, {
...options,
Copy link
Member

@sosukesuzuki sosukesuzuki May 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should use spread syntax for this because copying is little expensive. Can we use options.cursorOffset = -1?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, this option is user input, we shouldn't modify it. It can also be undefined.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think clone an object is that expensive.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, we can keep it.

@fisker fisker merged commit f105e01 into prettier:next May 8, 2022
@fisker fisker deleted the format-check branch May 8, 2022 09:10
medikoo pushed a commit to medikoo/prettier-elastic that referenced this pull request Jan 16, 2024
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