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

Indent <script> and <style> tags’ content in *.vue files #5574

Closed
wants to merge 2 commits into from
Closed

Indent <script> and <style> tags’ content in *.vue files #5574

wants to merge 2 commits into from

Conversation

nakatanakatana
Copy link

@nakatanakatana nakatanakatana commented Nov 28, 2018

Fixes #3888

  • 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)
  • I’ve read the contributing guidelines.

Try the playground for this PR

ikatyang
ikatyang previously approved these changes Nov 29, 2018
@ikatyang ikatyang added this to the 1.16 milestone Nov 29, 2018
@Kocal
Copy link
Contributor

Kocal commented Dec 5, 2018

Is it configurable with vue/script-indent, or no matter what <script> and <style> will be indented? 😕

@j-f1
Copy link
Member

j-f1 commented Dec 5, 2018

@Kocal <script> & <style> will always be indented no matter what.

@Kocal
Copy link
Contributor

Kocal commented Dec 5, 2018

Hum okay.

Copy link
Member

@duailibe duailibe left a comment

Choose a reason for hiding this comment

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

I have no opinion on this.. we just need to choose between people complaining that it's indented vs people complaining that it's NOT indented..

🤷‍♂️

@chrisvfritz
Copy link

As a member of the Vue team, I personally don't think this should have anything to do with Vue. There does seem to be a huge split on people who prefer an indent or not for embedded languages, but this isn't a Vue-specific thing. It should behave the same everywhere there's XML.

Copy link
Member

@lipis lipis left a comment

Choose a reason for hiding this comment

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

🍻 🍾

@chrisvfritz
Copy link

chrisvfritz commented Dec 7, 2018

To end bikeshedding on XML indentation for good, maybe it'd be best to just have an option like:

"htmlNoIndentTags": ["html", "script", "style"]

Then indentation behavior could be the same across XML everywhere, even .vue files, maintaining the current default to indent inside every tag since it's the conceptually simplest option.

@j-f1 j-f1 changed the title add indent script and style tags content in *.vue files Indent <script> and <style> tags content in *.vue files Dec 7, 2018
@j-f1 j-f1 changed the title Indent <script> and <style> tags content in *.vue files Indent <script> and <style> tags’ content in *.vue files Dec 7, 2018
@lydell
Copy link
Member

lydell commented Dec 8, 2018

To end bikeshedding on XML indentation for good, maybe it'd be best to just have an option

No, then we'd just end the bikeshedding in this repo, moving the bikeshedding to every team using Prettier. See our option philosophy.

@ikatyang ikatyang removed this from the 1.16 milestone Dec 14, 2018
@ikatyang ikatyang dismissed their stale review December 14, 2018 06:28

highly controversial

@chrisvfritz
Copy link

@lydell I'm familiar with the option philosophy and one of the cases when you add new configuration is when there's huge demand. I completely understand having to draw a line, so what would sufficiently demonstrate that demand for this is huge enough? In the past, it looks like configuration requests with 30-100 upvotes have been accepted and the issue to resist adding configuration currently has 426 upvotes.

@chrisvfritz
Copy link

chrisvfritz commented Dec 14, 2018

As an aside, my understanding is that another of Prettier's primary goals is consistency (and like any project, maintainability). As an open source maintainer myself, my worry is that special rules for .vue files set a precedent for more file-specific formatting. This would not only be a pain to implement and maintain, but also potentially confuse users. Something I really don't want is the same languages formatted differently depending on whether the extension is js, jsx, ts, tsx, vue, ejs, erb, etc - especially when the same extensions are used by multiple communities.

Copy link
Member

@ikatyang ikatyang left a comment

Choose a reason for hiding this comment

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

Based on the discussion in #3888, we're going to add an option for it and defaults to not indent to avoid unnecessary breaking change.

Could you add an option (maybe --vue-script-indent) for it and document the change in CHANGELOG.unreleased.md?

@ikatyang ikatyang added this to the 1.17 milestone Feb 1, 2019
@lipis lipis added the lang:vue Issues affecting Vue label Feb 5, 2019
@lipis
Copy link
Member

lipis commented Feb 5, 2019

@nakatanakatana are you still around to add the option for this?

@lipis
Copy link
Member

lipis commented Feb 11, 2019

@prettier/core It looks like @nakatanakatana is not around.. :) Who can take or of that one?

@duailibe duailibe self-assigned this Feb 11, 2019
@duailibe
Copy link
Member

I can do this

@lipis
Copy link
Member

lipis commented Feb 11, 2019

@duailibe and maybe we can start preparing the blog post and the 1.17 release for that?

Copy link
Member

@azz azz left a comment

Choose a reason for hiding this comment

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

I'm confused, why is this Vue-specific?

The proposed option --vue-script-indent doesn't make any sense to me. I thought the issue was about indenting/not-indenting embedded languages in HTML, not just .vue files?

@azz azz requested a review from lydell March 10, 2019 09:47
@lydell
Copy link
Member

lydell commented Mar 10, 2019

@azz I agree, I think it makes the most sense to have an option for indent/no-indent for all <script> and <style> tags in any kind of HTML. I think it should default to indenting. That would keep the current default behavior for .html files, but change it for .vue. That should be fine, though?

Also, I’m closing this PR. I think it’s easier to start from scratch now.

@lydell lydell closed this Mar 10, 2019
@ikatyang ikatyang removed this from the 1.17 milestone Apr 12, 2019
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 11, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang:vue Issues affecting Vue locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Indent script and style tags content in *.vue files
9 participants