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

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

Closed
bminer opened this issue Nov 17, 2020 · 11 comments · Fixed by #159
Closed

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

bminer opened this issue Nov 17, 2020 · 11 comments · Fixed by #159
Assignees
Labels
type: enhancement Functionality that enhances existing features

Comments

@bminer
Copy link

bminer commented Nov 17, 2020

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
Copy link
Author

bminer commented Nov 17, 2020

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 Shinigami92 self-assigned this Nov 17, 2020
@Shinigami92 Shinigami92 added the type: enhancement Functionality that enhances existing features label Nov 17, 2020
@Shinigami92
Copy link
Member

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

@Shinigami92
Copy link
Member

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 🙂 and I'll review it and help you. This could help get that improvement faster.

@bminer
Copy link
Author

bminer commented Nov 18, 2020

@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
Copy link
Member

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

plugin-pug/src/printer.ts

Lines 185 to 189 in 9db4489

let token: Token | null = this.getNextToken();
while (token) {
logger.debug('[PugPrinter]:', JSON.stringify(token));
try {
switch (token.type) {

In this line, every token will be passed and called by it's related function

results.push(this[token.type](token));


You are specially interested in

plugin-pug/src/printer.ts

Lines 1065 to 1067 in 9db4489

private dot(token: DotToken): string {
return '.';
}

and

plugin-pug/src/printer.ts

Lines 1046 to 1055 in 9db4489

private ['start-pipeless-text'](token: StartPipelessTextToken): string {
this.pipelessText = true;
return `\n${this.indentString.repeat(this.indentLevel)}`;
}
private ['end-pipeless-text'](token: EndPipelessTextToken): string {
this.pipelessText = false;
this.pipelessComment = false;
return '';
}

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
Copy link
Member

@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
Copy link
Author

bminer commented Nov 30, 2020

@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
Copy link
Author

bminer commented Nov 30, 2020

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Functionality that enhances existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants