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

Vue: Support custom blocks #8023

Merged
merged 24 commits into from Apr 16, 2020

Conversation

sosukesuzuki
Copy link
Member

@sosukesuzuki sosukesuzuki commented Apr 11, 2020

This improves formatting custom blocks in Vue SFC.(see #5448 and #5458)

If there is lang attribute, the value of lang is passed to parser option.(If the value of lang is unknown, no formatting is done)

<!-- Input -->
<custom lang="babel">
const foo =
    'foo'
</custom>

<!-- Output -->
<custom lang="babel">
const foo = "foo";
</custom>

real world custom blocks examples:

How do you think?

  • I’ve added tests to confirm my change works.
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@fisker
Copy link
Member

fisker commented Apr 11, 2020

I guess we still can't parse this

<custom lang="babel">
const foo = "</";
</custom>

@sosukesuzuki
Copy link
Member Author

@fisker Yes, I think we should fix the problem in other way like #7975

@fisker
Copy link
Member

fisker commented Apr 11, 2020

@sosukesuzuki
I think we should add logic here,

function inferScriptParser(node) {

maybe also both lang and type? I don't think people would like to write lang="json-stringify", should be lang="json" or type="application/json".

for (let i = 0; i < parsers.length && !printed; i++) {
const parser = parsers[i];
try {
printed = textToDoc(node.value, { parser });
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the idea guessing languages, people even complaining about auto format with known language.

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 like this too, but in the real world custom blocks are often used without attributes.

Copy link
Member

Choose a reason for hiding this comment

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

In that way, why don't we ignore it. Want format? Tell me what it is.

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 think formatting for custom blocks is needed. For example, eslint-plugin-gridsome and eslint-plugin-prettier-vue use Prettier to format custom blocks.
Could you tell me what is the reason for ignoring it?

Copy link
Member

@fisker fisker Apr 12, 2020

Choose a reason for hiding this comment

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

I'm not saying disable formatting custom blocks.
I'm worried about wrong results.

We should ignore

<docs></docs>

But format

<docs lang="markdown"></docs>

Maybe with some exception like <page-query> known as graphql block, just like <script> in html we assume that's javascript

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 understood. What do you think about adding options for custom blocks like eslint-plugin-prettier-vue? (I'm reluctant to add options)

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'll remove guessing languages for now. At least for this PR, it only supports custom blocks using the lang / type attribute.

Copy link
Member

Choose a reason for hiding this comment

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

👍 , I guess we can use --embedded-language-formatting later?

Copy link
Member Author

Choose a reason for hiding this comment

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

sounds good.

Comment on lines 198 to 209
function stripTrailingHardline(doc) {
// HACK remove ending hardline, original PR: #1984
if (doc.type === "concat" && doc.parts.length !== 0) {
const lastPart = doc.parts[doc.parts.length - 1];
const parts = getInnerParts(doc);
const lastPart = parts[parts.length - 1];
if (lastPart.type === "concat") {
if (
lastPart.parts.length === 2 &&
lastPart.parts[0].hard &&
lastPart.parts[1].type === "break-parent"
) {
return { type: "concat", parts: doc.parts.slice(0, -1) };
return { type: "concat", parts: parts.slice(0, -1) };
Copy link
Member

Choose a reason for hiding this comment

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

Potential breaking change for plugins

Copy link
Member

Choose a reason for hiding this comment

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

I think we can implement the withInnerParts argument (and maybe put todo to change it on default for prettier@3)

/cc @thorn0

Copy link
Member

Choose a reason for hiding this comment

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

Are doc-utils considered part of the public plugin API?

src/language-html/utils.js Outdated Show resolved Hide resolved
Copy link
Member

@alexander-akait alexander-akait 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, but want some feedback from @thorn0 about changes in doc-utils

@fisker
Copy link
Member

fisker commented Apr 13, 2020

Just want let you know, I tried use @vue/component-compiler-utils to parse .vue today, can't get comments at root and can't keep attribute orders, no info in the SFCDescriptor . fisker#347

src/document/doc-utils.js Outdated Show resolved Hide resolved
@thorn0
Copy link
Member

thorn0 commented Apr 13, 2020

Prettier pr-8023
Playground link

--parser vue

Input:

<custom lang="zzz">
const foo = "foo";
  const foo = "foo";</custom>

Output:

<custom lang="zzz"> const foo = "foo";
  const foo = "foo";>

@fisker

This comment has been minimized.

@fisker

This comment has been minimized.

Copy link
Member

@thorn0 thorn0 left a comment

Choose a reason for hiding this comment

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

  1. Vue: Support custom blocks #8023 (comment)
  2. lang="babel" should not be recognized. lang="javascript" should. babel is not a language, it's Prettier's implementation detail. Check how the language recognition is implemented for code blocks in Markdown.
  3. --vue-indent-script-and-style shouldn't affect custom block whose language wasn't recognized.

@fisker
Copy link
Member

fisker commented Apr 13, 2020

Prettier pr-8023
Playground link

--parser vue

Input:

<custom lang="json">{
  "a": 1
}</custom>

<custom lang="json">{
  "a": 1
}
</custom>

Output:

<custom lang="json"
{
  "a": 1
}
>

<custom lang="json"
{
  "a": 1
}
</custom>

Copy link
Member

@thorn0 thorn0 left a comment

Choose a reason for hiding this comment

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

  • Blocks with unknown languages must be preserved as is.

    Prettier pr-8023
    Playground link

     --parser vue

    Input:

     <custom lang="zzz">123</custom>

    Output:

     <custom lang="zzz">
     123
     </custom>
    
  • Please merge master or rebase to see how this will work with --embedded-language-formatting=off.

@sosukesuzuki
Copy link
Member Author

I'm busy this week, so I'll continue working on it over the weekend.

@sosukesuzuki
Copy link
Member Author

I worked on this because it wasn't as busy as I thought it would be.

@thorn0

This comment has been minimized.

@thorn0
Copy link
Member

thorn0 commented Apr 15, 2020

What about this point? Why can that code be parsed in <script>...</script> but not in custom blocks? Can we parse custom blocks the same way we parse script tags?

@sosukesuzuki
Copy link
Member Author

sosukesuzuki commented Apr 15, 2020

@thorn0 Probably, we cannot fix it from Prettier side. angular-html-parser seems to parse <script>... </script> in a different way from <custom>...</custom>.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants