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

Clean rendered html output #449

Merged

Conversation

gael-boyenval
Copy link
Contributor

@gael-boyenval gael-boyenval commented Aug 24, 2016

Addresses #448

Summary of changes:

  • add js-beautify dependency
  • add clean into the render function

@bmuenzenmeyer
that's surely not the right place to do it. the tests are failing.
just for you too see.

maybe it would be more suited to modify pattern_assembler.renderPattern ?

@bmuenzenmeyer
Copy link
Member

@gael-boyenval I think this could likely be moved to just before the writes.

Like around https://github.com/pattern-lab/patternlab-node/blob/master/core/lib/patternlab.js#L388-L395

@gael-boyenval gael-boyenval force-pushed the clean-rendered-html-output branch from 9f8eaf6 to b27a5fb Compare September 2, 2016 12:21
@gael-boyenval gael-boyenval force-pushed the clean-rendered-html-output branch from b27a5fb to a6f4d9c Compare September 2, 2016 12:27
Copy link
Member

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

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

I pulled this down and have tested.
everything so far looks great! HTML is formatted better than nothing.

I see that the js-beautify library seems to accept an .editorconfig file via CLI and perhaps options. I wonder if we can get this to work via something like https://github.com/editorconfig/editorconfig-core-js

I didn't try it, but it'd be nice if the template code could be beautified too. It looks like you omitted that (because I presume, you don't know exactly which file we would be dealing with.) I think since most template languages are html-esque, it might work?

@bmuenzenmeyer bmuenzenmeyer merged commit add7776 into pattern-lab:dev Sep 23, 2016
@gael-boyenval
Copy link
Contributor Author

gael-boyenval commented Sep 23, 2016

@bmuenzenmeyer : sorry i didn't saw th message earlier.
Regarding the templates, i think that the user has full hand on the output, since nothing is transformed, it's just copied.
I assumed that the user would want to keep them as they were created.
(assuming you are talking about pattern templates… yes ?)

@bmuenzenmeyer
Copy link
Member

@gael-boyenval You'll see in my additional commit (now on dev!) that I am beautifying templates too.

I plan to release 2.6.0 as an alpha, so I think we can give it a go for a bit and see if there are any problems.

does that sound ok?

@bmuenzenmeyer
Copy link
Member

sorry i didn't saw the message earlier.

and this was my first time using the new review interface, so I didn't know if it'd be the same as @mentioning you

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

Successfully merging this pull request may close these issues.

2 participants