prettier / plugin-pug

Prettier Pug Plugin
https://prettier.github.io/plugin-pug
MIT License
199 stars 43 forks source link

Bug: Inline JavaScript in text block is not pretty #158

Closed bminer closed 4 years ago

bminer commented 4 years ago

It seems that Pug script. blocks aren't parsed / printed properly. It should make the JavaScript code prettier.

Info

Tool Version
Plugin v1.10.1
Prettier v2.1.2
Framework none
Node v14.15.0
OS linux

Prettier config

{
    "useTabs": true,
    "semi": false,
    "arrowParens": "avoid",
    "trailingComma": "es5",
    "overrides": [{
        "files": "*.riot",
        "options": {"parser": "pug"}
    }]
}

Just ignore what I'm doing with *.riot file extensions. This bug exists even when --no-config flag is passed to prettier.

Input

script.
    const   x   =   2   ;   // comment

Output or Error

cat test.pug | npx prettier --parser pug
script.
    const   x   =   2   ;   // comment

Expected Output

script.
    const x = 2 // comment

Compare this to Prettier's HTML parser:

cat | npx prettier --parser html
<script>
    const   x   =   2   ;   // comment
</script>

which outputs

<script>
    const x = 2 // comment
</script>

Notice missing semicolon and proper spacing

bminer commented 4 years ago

HTML parser in Prettier does something a bit more complex: https://github.com/prettier/prettier/blob/master/src/language-html/printer-html.js#L109

Shinigami92 commented 4 years ago

I think I know how I can solve this Hopefully I can work on it in the evening, but I'm not sure

Shinigami92 commented 4 years ago

Ok, I looked in the token stream and see that I need to iterate over, bundle and format with JS everything between the start-pipeless-text and end-pipeless-text if it is based on a script-tag. But I don't have my head for it right now and it could take a while. Sorry to you that I am not making this such a high priority because it doesn't break anyone's code or is a stopper, just an improvement. You can currently format it by hand until I have formatted this section. Maybe this week, maybe this weekend.

If you want you can try creating a PR yourself :slightly_smiling_face: and I'll review it and help you. This could help get that improvement faster.

bminer commented 4 years ago

@Shinigami92 - Sure, I'd love to help where I can. Sadly, I don't think I understand Prettier enough to know where to go from here. How do you switch the parser from "pug" to "babel" or something else? I'm sure I could study Prettier's built-in HTML parser a bit more... but it looks tricky... and very different from the pug parser here.

What do you think?

Shinigami92 commented 4 years ago

Oh, IMO, it's a lot easier than you think :)

First, you should start reading the CONTRIBUTING.md

Then you should create a folder tests/issues/issue-158 and copy over 3 files from e.g. issue-147 (that's at least the easiest currently) Modify the un-/formatted.pug files and the test description (test('should xy'))

Now you are interested in src/printer.ts Here all the pug file was splitted into several so called tokens https://github.com/prettier/plugin-pug/blob/9db44894a54861150053f8a4e5f6ddd942060769/src/printer.ts#L185-L189

In this line, every token will be passed and called by it's related function https://github.com/prettier/plugin-pug/blob/9db44894a54861150053f8a4e5f6ddd942060769/src/printer.ts#L215


You are specially interested in https://github.com/prettier/plugin-pug/blob/9db44894a54861150053f8a4e5f6ddd942060769/src/printer.ts#L1065-L1067 and https://github.com/prettier/plugin-pug/blob/9db44894a54861150053f8a4e5f6ddd942060769/src/printer.ts#L1046-L1055

So you will get a token-stream for e.g.

script.
  const   x   =   2   ;   // comment

script.
  if (usingPug)
    console.log('you are awesome')
  else
    console.log('use pug')
[
  {"type":"tag","loc":{"start":{"line":1,"column":1},"end":{"line":1,"column":7}},"val":"script"},
  {"type":"dot","loc":{"start":{"line":1,"column":7},"end":{"line":1,"column":8}}},
  {"type":"start-pipeless-text","loc":{"start":{"line":1,"column":8},"end":{"line":1,"column":8}}},
  {"type":"text","loc":{"start":{"line":2,"column":3},"end":{"line":2,"column":37}},"val":"const   x   =   2   ;   // comment"},
  {"type":"newline","loc":{"start":{"line":3,"column":1},"end":{"line":3,"column":1}}},
  {"type":"text","loc":{"start":{"line":3,"column":1},"end":{"line":3,"column":1}},"val":""},
  {"type":"end-pipeless-text","loc":{"start":{"line":3,"column":1},"end":{"line":3,"column":1}}},
  {"type":"newline","loc":{"start":{"line":4,"column":1},"end":{"line":4,"column":1}}},
  {"type":"tag","loc":{"start":{"line":4,"column":1},"end":{"line":4,"column":7}},"val":"script"},
  {"type":"dot","loc":{"start":{"line":4,"column":7},"end":{"line":4,"column":8}}},
  {"type":"start-pipeless-text","loc":{"start":{"line":4,"column":8},"end":{"line":4,"column":8}}},
  {"type":"text","loc":{"start":{"line":5,"column":3},"end":{"line":5,"column":16}},"val":"if (usingPug)"},
  {"type":"newline","loc":{"start":{"line":6,"column":1},"end":{"line":6,"column":3}}},
  {"type":"text","loc":{"start":{"line":6,"column":3},"end":{"line":6,"column":35}},"val":"  console.log('you are awesome')"},
  {"type":"newline","loc":{"start":{"line":7,"column":1},"end":{"line":7,"column":3}}},
  {"type":"text","loc":{"start":{"line":7,"column":3},"end":{"line":7,"column":7}},"val":"else"},
  {"type":"newline","loc":{"start":{"line":8,"column":1},"end":{"line":8,"column":3}}},
  {"type":"text","loc":{"start":{"line":8,"column":3},"end":{"line":8,"column":27}},"val":"  console.log('use pug')"},
  {"type":"end-pipeless-text","loc":{"start":{"line":8,"column":27},"end":{"line":8,"column":27}}},
  {"type":"eos","loc":{"start":{"line":8,"column":27},"end":{"line":8,"column":27}}}
]

You see the sections between start-pipeless-text and end-pipeless-text? These are the tokens that you need to handle/process. So you could join their values together and pass them to the format function of prettier (format(text, { parser, ... })) and/or handle them together with a boolean flag in their related function calls. You also need to manage the currentIndex and maybe also currentLineLength and the indentation etc.

Shinigami92 commented 4 years ago

@bminer Good news: I've completed the basic work on this enhancement During the next few days I'll take a look at what I've written to identify more potential issues and possible improvements

bminer commented 4 years ago

@Shinigami92 - Looks great! Thanks! Inline scripts now run through prettier, but the inline script output does not respect the .prettierrc configuration file. Furthermore, the indentation within the pug file should probably be consistent with the inline script; that is, if the pug file is tab-indented, the inline JavaScript should also be tab-intented.

bminer commented 4 years ago

Seems to work great if you merge options into codeInterpolationOptions. Not sure why code interpolation options overrides the singleQuote flag for Prettier as shown here. Shall I open another issue for this? I think we are VERY close!

Shinigami92 commented 4 years ago

The problem with the singleQuotes is more a problem of #45
There is a huge problem with that, because of detection of some evil string quotes like "`' and their escaption

That's why it's always the opposite of pugSingleQuotes option

Maybe we can improve it for some code parts like when it's not in an attribute But two things:

  1. It needs a huge load of tests and we need to find the correct places where it's possible and where not
  2. It will be a breaking change (cause it will flip every quotes of these codes)
Shinigami92 commented 4 years ago

Ah yeah and another problem: if we want to support the current behaviour, we need to introduce a kinda third option pugJsSingleQuotes IMO that's overkill 😕

Shinigami92 commented 4 years ago

I would suggest to checkout the repo and use yarn link + yarn link "@prettier/plugin-pug" in your target project and experiment a bit with that Also you can run yarn test --silent and see the problems

If you found a solution, please create a PR