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

Prettier not respecting prettier-ignore-start and prettier-ignore-end #5557

Open
lamyergeier opened this issue Nov 26, 2018 · 17 comments
Open
Labels
area:ignore .prettierignore file, --ignore-path CLI option, /* prettier-ignore */ comments and so on area:ranges Issues about formatting/ignoring/etc segments of files lang:markdown Issues affecting Markdown type:enhancement A potential new feature to be added, or an improvement to how we print something

Comments

@lamyergeier
Copy link

Prettier 1.15.2
Playground link

--parser markdown
--prose-wrap never

Input:

- **Sequence of initialization**:
<!-- prettier-ignore-start -->
  #. Firmware
  #. Bootloader (GNU Grub)
  #. Operating System Kernel
  #. Service Manager (e.g. `systemd`)
<!-- prettier-ignore-end -->

Output:

- **Sequence of initialization**:
  <!-- prettier-ignore-start -->
  #. Firmware #. Bootloader (GNU Grub) #. Operating System Kernel #. Service Manager (e.g. `systemd`)
  <!-- prettier-ignore-end -->

Expected behavior:
See Input

@j-f1 j-f1 added type:bug Issues identifying ugly output, or a defect in the program lang:markdown Issues affecting Markdown labels Nov 26, 2018
@jaideng123
Copy link
Contributor

I can look into this

@jaideng123
Copy link
Contributor

jaideng123 commented Dec 2, 2018

This is occurring because the only time we check for prettier-ignore start and end is on the children of the root node (which in this case would be [List]) and not in any of the deeper nodes. I'll start figuring out how to fix it

@jaideng123
Copy link
Contributor

jaideng123 commented Dec 2, 2018

I checked the original PR for the code (#4202) and it looks like when it was merged in the intention was to only support top-level ignore-start/end so this isn't necessarily a bug but rather missing functionality
@j-f1 Thoughts?

@j-f1 j-f1 added type:enhancement A potential new feature to be added, or an improvement to how we print something and removed type:bug Issues identifying ugly output, or a defect in the program labels Dec 2, 2018
@j-f1
Copy link
Member

j-f1 commented Dec 2, 2018

Seems like your evaluation is correct. If you’d like to add support for the comments in more places, feel free!

@jaideng123
Copy link
Contributor

I'll take a crack at it

@duailibe
Copy link
Member

duailibe commented Dec 4, 2018

I'm not sure we want to support it in more places. Ignore comment is intended to be a escape hatch, not a feature we want to extend

@j-f1 j-f1 removed the lang:markdown Issues affecting Markdown label Dec 4, 2018
@ikatyang
Copy link
Member

ikatyang commented Dec 5, 2018

@anishmittal2020 It's already possible to ignore them, is this what you want?

Prettier 1.15.3
Playground link

--parser markdown
--prose-wrap never

Input:

- **Sequence of initialization**:

  <!-- prettier-ignore -->
  #. Firmware
  #. Bootloader (GNU Grub)
  #. Operating System Kernel
  #. Service Manager (e.g. `systemd`)
  
  __formatting still works here__

Output:

- **Sequence of initialization**:

  <!-- prettier-ignore -->
  #. Firmware
  #. Bootloader (GNU Grub)
  #. Operating System Kernel
  #. Service Manager (e.g. `systemd`)

  **formatting still works here**

@ikatyang ikatyang added the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Dec 5, 2018
@lamyergeier
Copy link
Author

lamyergeier commented Dec 5, 2018

@ikatyang

  • this works but do we need to give 2 blank spaces before <!-- prettier-ignore --> for it to work?
  • Also, IMHO <!-- prettier-ignore-start --> and <!-- prettier-ignore-end --> is a better option as it is very explicitly clear what part should be ignored. If we use <!-- prettier-ignore --> then we need to use it every time we have a blank line in a range.

Summary:

  • IMHO Range ignore seems to be more semantically clear and the file looks cleaner. Is more elegant.
  • If a user uses range ignore, he can be mentally sure that the range should be ignored no matter how the content in the range is formatted (i.e. with or without blank lines).

Prettier 1.15.3
Playground link

--parser markdown
--prose-wrap never

Input:

  <!-- prettier-ignore -->
  a) regular use
    1. finish something worthy of a commit
    2. commit
  b) independent fixup
    1. realize that something does not work
    2. fix that
    3. commit it

Output:

  <!-- prettier-ignore -->

a) regular use 1. finish something worthy of a commit 2. commit b) independent fixup 1. realize that something does not work 2. fix that 3. commit it

Expected behavior: See Input


NOTE: The following Input itself has not been rendered correctly here, please go to the playground link

Prettier 1.15.3
Playground link

--parser markdown
--prose-wrap never

Input:

- Workflow
  <!-- prettier-ignore -->
  a) regular use
  <!-- prettier-ignore -->
	1. finish something worthy of a commit
    2. commit
  <!-- prettier-ignore -->
  b) independent fixup
  <!-- prettier-ignore -->
    1. realize that something does not work
    2. fix that
    3. commit it

Output:

- Workflow
  <!-- prettier-ignore -->
  a) regular use
  <!-- prettier-ignore -->
  
	1. finish something worthy of a commit
  2. commit
     <!-- prettier-ignore -->
     b) independent fixup
     <!-- prettier-ignore -->
  1. realize that something does not work
  1. fix that
  1. commit it

Expected behavior: See Input

@no-response no-response bot removed the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Dec 5, 2018
@ikatyang
Copy link
Member

ikatyang commented Dec 5, 2018

It seems you'd like to use pandoc-specific syntax in some places but the rest of them are still valid CommonMark. I think it'd be better to find a remark plugin to support pandoc instead of using a lot of range ignore but I'm not sure if there's one.

@ikatyang ikatyang added the lang:markdown Issues affecting Markdown label Dec 5, 2018
@lamyergeier
Copy link
Author

lamyergeier commented Dec 5, 2018

Yes, if we could do prettier ignore for a range at any place in the document, it will be great. I think many people would like that, as I think many people might use other standard (like pandoc) for certain things rarely, but mostly they follow commonmark.

I understand from @duailibe comment that Prettier want's to enforce standard and weed out nonstandard things. IMHO: In the case of source code, people use prettier to standardize code that can be shared. But in the case of markdown usually, people share the generated HTML or PDF and not markdown. So, I think that this feature can be extended to markdown and limited only to Markdown and not to other language like Javascript etc. If people could use some non-standard markdown, it should be ok as they don't share the source or the markdown file itself.

Also, sensible people won't misuse prettier-ignore as they would use it rarely because using this at all the places is as good as not using prettier at all.

@jaideng123
Copy link
Contributor

Life has gotten a little busy for me recently so I'm going to step off this issue and open it up for anyone else that wants to work on it

@jimmyhoran
Copy link

jimmyhoran commented Jan 28, 2019

I think @anishmittal2020 is onto a great suggestion here. Allowing an explicit range ignore anywhere in any prettier supported file would be very helpful. This kind of implementation is present in many other platform linters and formatters.
If you try this out in a prettier playground the range ignore is being ignored 🙈

<!-- prettier-ignore-start -->
<div  class="dark-background">
  </div>
<div class='light-background'></div>
<a href=""/>
<!-- prettier-ignore-end -->

As mentioned above inserting <-- prettier-ignore --> multiple times is not a very nice implementation.
A common example would be partials in a HTML file where I would need to do the following...

<!-- prettier-ignore -->
{{ define "title" }}
<!-- prettier-ignore -->
{{ .Title }} &ndash; {{ .Site.Title }}
<!-- prettier-ignore -->
{{ end }}
<!-- prettier-ignore -->
{{ define "main" }}
<!-- prettier-ignore -->
{{ .Content }}
<!-- prettier-ignore -->
{{ end }}

<!-- The rest of my HTML I would like to be formatted -->

Would be much cleaner to be able to do the this...

<!-- prettier-ignore-start -->
{{ define "title" }}
{{ .Title }} &ndash; {{ .Site.Title }}
{{ end }}
{{ define "main" }}
{{ .Content }}
{{ end }}
<!-- prettier-ignore-end -->

<!-- The rest of my HTML I would like to be formatted -->

Is this being worked on by anyone?

@thorn0 thorn0 added the area:ranges Issues about formatting/ignoring/etc segments of files label Nov 23, 2019
@thorn0 thorn0 added the area:ignore .prettierignore file, --ignore-path CLI option, /* prettier-ignore */ comments and so on label May 6, 2020
@pawamoy
Copy link

pawamoy commented Sep 27, 2020

@jimmyhoran

If you try this out in a prettier playground the range ignore is being ignored 🙈

<!-- prettier-ignore-start -->
<div  class="dark-background">
  </div>
<div class='light-background'></div>
<a href=""/>
<!-- prettier-ignore-end -->

It should actually work if you add a blank line before the end tag:

<!-- prettier-ignore-start -->
<div  class="dark-background">
  </div>
<div class='light-background'></div>
<a href=""/>

<!-- prettier-ignore-end -->

At least that's the behavior I get in this Prettier playground.

@thorn0
Copy link
Member

thorn0 commented Sep 28, 2020

See also the explanation in #7027 (comment)

@reduardo7
Copy link

@mrwensveen
Copy link

@jimmyhoran The request to support ranges in any language is here: #5287

@yermulnik
Copy link

It should actually work if you add a blank line before the end tag:

Lifesaver. Thanks a bunch @pawamoy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:ignore .prettierignore file, --ignore-path CLI option, /* prettier-ignore */ comments and so on area:ranges Issues about formatting/ignoring/etc segments of files lang:markdown Issues affecting Markdown type:enhancement A potential new feature to be added, or an improvement to how we print something
Projects
None yet
Development

No branches or pull requests