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

Clear prettier error with prettier-ignore comment #3330

Merged
merged 1 commit into from
Mar 3, 2022

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Mar 3, 2022

* stop prettier from incorrectly formatting line in command-ref/repro
@shcheklein shcheklein temporarily deployed to dvc-org-fix-prettier-wa-ne47ei March 3, 2022 12:51 Inactive
@julieg18 julieg18 self-assigned this Mar 3, 2022
@@ -51,6 +51,7 @@ There are a few ways to restrict what will be regenerated by this command: by
specifying specific reproduction [`targets`](#options), or by using certain
command [options](#options), such as `--single-item` or `--all-pipelines`.

<!-- prettier-ignore -->
> Note that stages without dependencies nor outputs are considered [always
Copy link
Contributor Author

@julieg18 julieg18 Mar 3, 2022

Choose a reason for hiding this comment

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

If we don't add a comment, prettier adds a > at the end of line 55.

@julieg18 julieg18 requested review from a team and removed request for a team March 3, 2022 13:02
Copy link
Contributor

@yathomasi yathomasi left a comment

Choose a reason for hiding this comment

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

Looks good to me.

This unique issue seems to happen when the bracketed link falls on line wrap. So, I don't see a way to fix this on a prettier config.
Changing proseWrap would affect other.

@julieg18 julieg18 merged commit 3aab5e7 into master Mar 3, 2022
@julieg18 julieg18 deleted the fix-prettier-warning branch March 3, 2022 17:09
@shcheklein
Copy link
Member

Hmm. Restyle is using prettier also. pre-commit and Restyle must be consistent. If they are not- that'a a bug to my mind. Or am I missing something?

We've never so far had to add prettier-ignore as far as I remember.

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Mar 3, 2022

pre-commit and Restyle must be consistent

They are. Prettier on the pre-commit hook tries to do the same. I had to git commit -n in #3327 (comment).

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Mar 3, 2022

This unique issue seems to happen when the bracketed link falls on line wrap

Is that somehow valid or should we maybe report the issue to https://github.com/prettier/prettier ?

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Mar 3, 2022

We've never so far had to add prettier-ignore as far as I remember.

p.s. we only have one more around a YAML block in https://raw.githubusercontent.com/iterative/dvc.org/master/content/docs/user-guide/project-structure/pipelines-files.md

@shcheklein
Copy link
Member

They are.

Then I might be missing the problem. Could you give me some context? If they are consistent we should never see Retyled PRs if pre-commit got applied, right?

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Mar 3, 2022

Correct. Full context:

  1. Prettier wands to add a > symbol at the end of the line in an edge case situation, which seems like a bug.
  2. To avoid that I forcibly skipped the hook locally (in 4f8f889), but Restyled created its PR since Prettier failed upstream.
  3. I Ignored the Restyled PR and merged the original (which closed the Restyled one), thinking we wouldn't have to worry about it after that PR.
  4. Because the GH checks actually check formatting for the whole code base now (since format: lint all files #2624), merging that line actually caused every PR after to create Restyled ones.
  5. This PR tells Prettier to ignore the line to avoid 4.

@shcheklein
Copy link
Member

Thanks, Jorge. Okay, I see they are aligned and both of them have the same bug. Have we raised the bug in the prettier repo to track? Also, can we put a comment with a link to this discussion somewhere nearby the ignore pragma?

@julieg18
Copy link
Contributor Author

julieg18 commented Mar 3, 2022

Thanks, Jorge. Okay, I see they are aligned and both of them have the same bug. Have we raised the bug in the prettier repo to track? Also, can we put a comment with a link to this discussion somewhere nearby the ignore pragma?

Looks like there is already an issue opened! There is also a pr opened for it already as well :)

@jorgeorpinel
Copy link
Contributor

Great. I subscribed to the issue for now.

@casperdcl
Copy link
Contributor

The PR's been open since Sept 2020 (prettier/prettier#9125) so prob not being merged soon.

iesahin pushed a commit that referenced this pull request Apr 11, 2022
* stop prettier from incorrectly formatting line in command-ref/repro
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.

5 participants