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

Add pugClassNotation 'attribute' option #203

Merged
merged 5 commits into from
Apr 23, 2021

Conversation

homerjam
Copy link
Contributor

This is a partial fix for #167 covering classes, I think it should be fairly straightforward to implement for IDs next, please let me know if this direction is right and I'll get on to it!

Thanks!

@homerjam homerjam requested a review from Shinigami92 as a code owner April 21, 2021 16:28
@Shinigami92 Shinigami92 added the type: feature request Functionality that introduces a new feature label Apr 22, 2021
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Code is looking good until now 👍
You can proceed like this
Do you want to handle id in another PR? Or currently just solve it for class?

src/printer.ts Outdated Show resolved Hide resolved
src/printer.ts Outdated Show resolved Hide resolved
@homerjam
Copy link
Contributor Author

You know what, a separate PR would be better. It's classes I really need and I'm not sure I'll get on to IDs soon. Hopefully this helps anyone else wanting to have a go at it!

@Shinigami92
Copy link
Member

Could you update https://github.com/prettier/plugin-pug/blob/main/docs/guide/pug-specific-options.md
and add a test case that contains class with js code

e.g.

- var clazz = 'ma-2 px-1'
div(class=clazz)

@homerjam homerjam force-pushed the literals-to-attributes branch from c80ff50 to ec27266 Compare April 22, 2021 16:19
@homerjam homerjam changed the title Add pugClassNotation: 'attribute' option (#167) Add pugClassNotation: 'attribute' option Apr 22, 2021
@homerjam
Copy link
Contributor Author

I just broke my rebase cherry 😊

I hope you didn't want any of that whitespace in the guide, sorry about that.

@Shinigami92
Copy link
Member

Ah, these whitespaces are necessary cause otherwise VuePress will render these in one line instead of adding a newline

@homerjam
Copy link
Contributor Author

Oh no! I have an overzealous markdown formatter I'm afraid. Whitespace is restored now...

Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Cause of work for my employer, I hope I can setup and bundle everything together this evening or over the weekend

@@ -773,6 +775,20 @@ export class PugPrinter {
if (token.val !== true) {
this.result += `=${token.val}`;
}
} else if (token.name === 'class' && this.options.pugClassNotation === 'attribute') {
const val: string = isQuoted(token.val) ? token.val.slice(1, -1).trim() : token.val;
const classes: string[] = val.split(/\s+/);
Copy link
Member

Choose a reason for hiding this comment

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

thought: I'm a little bit afraid of this line and don't know what it's doing if the class contains js code with spaces 🤔
But I think I will merge it for now

@Shinigami92 Shinigami92 changed the title Add pugClassNotation: 'attribute' option Add pugClassNotation 'attribute' option Apr 23, 2021
@Shinigami92 Shinigami92 merged commit 063d326 into prettier:main Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request Functionality that introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants