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

Format CSS on push to master #88

Closed
AObuchow opened this issue Jun 25, 2020 · 18 comments
Closed

Format CSS on push to master #88

AObuchow opened this issue Jun 25, 2020 · 18 comments
Labels

Comments

@AObuchow
Copy link
Owner

AObuchow commented Jun 25, 2020

It'd be nice to have some automatic CSS formatting running with GitHub Actions when pushing to master.

I haven't looked much into the topic, but I found these CSS formatters that could be useful:

@AObuchow
Copy link
Owner Author

Ideally, it'd be very nice to be able to format Eclipse CSS properties, eg. https://github.com/AObuchow/Eclipse-Spectrum-Theme/blob/master/com.aobuchow.themes.spectrum/css/preference_styles.css. Doing manual formating on these types of files is quite tedious.

@AObuchow
Copy link
Owner Author

@ingomohr this might be something that interests you ;)

@AObuchow
Copy link
Owner Author

Potentially (?) relevant: https://github.com/marketplace/actions/auto-minify

@AObuchow
Copy link
Owner Author

@tabjy do you happen to have any preffered CSS formater tools?

@tabjy
Copy link

tabjy commented Jun 26, 2020

@tabjy do you happen to have any preffered CSS formater tools?

I often just use the preset comes with my editor/ide

@AObuchow
Copy link
Owner Author

@tabjy me too ;) Thanks for the input !!

@ingomohr
Copy link
Contributor

What about format-on-save (i.e. right when the file is saved)?
For JDT, there is such a feature. Maybe, there also is one for CSS?

@AObuchow
Copy link
Owner Author

@ingomohr Ill have to look into whether wild web developer supports formatting CSS (as it’s provided by a language server).

A github workflow would be nice however so that any contributors CSS additions could be auto formatted regardless of their editor/IDE.

@AObuchow
Copy link
Owner Author

To clarify, I edit Spectrum Theme’s CSS with Eclipse’s Generic Editor, which is getting LSP support from the CSS LS provided by Eclipse Wild Web Developer

@ingomohr
Copy link
Contributor

@ingomohr Ill have to look into whether wild web developer supports formatting CSS (as it’s provided by a language server).

A github workflow would be nice however so that any contributors CSS additions could be auto formatted regardless of their editor/IDE.

Yes, it would be nice to have the contributors not worry about formatting.
On the other hand, this would cause a new commit after the merge to master. Also, this would not help you for a review because the format would be corrected only after you've reviewed the pull request.

If you would set the format-on-save preference as project specific preference, the contributors would also not have to worry.

@AObuchow
Copy link
Owner Author

Yes, it would be nice to have the contributors not worry about formatting.
On the other hand, this would cause a new commit after the merge to master. Also, this would not help you for a review because the format would be corrected only after you've reviewed the pull request.

These are 2 good points that I hadn't considered. I don't mind the formatting not being visible at the review stage, however, maybe a better solution than a GitHub workflow exists (as you proposed below).

If you would set the format-on-save preference as project specific preference, the contributors would also not have to worry.

I suppose you mean .project/Eclipse specific settings correct? This is a good idea :)
However, ideally, I wouldn't want to limit contributors to using Eclipse (despite the fact it's CSS for an Eclipse theme...). Some people might be used to editing CSS in VSCode for example.

For JS/TS, ESLint which requires a .eslintrc which is a editor-agnostic configuration file for linting rules. ESLint can be run through IDE's (eg. Eclipse + Wild Web Developer) or as a standalone application. Maybe there's some CSS formatter application that has a similar configuration setup? PR guidelines could mention to run the CSS formatter before submitting a PR.

The more I think of this... the more I believe I'm overthinking it - your suggestion might be simplest :) (Provided that it's already supported in Wild Web Developer)

@ingomohr
Copy link
Contributor

All good points, you've made @AObuchow :).

We'll see what works best, I think.

@AObuchow
Copy link
Owner Author

AObuchow commented Jun 26, 2020

So, Wild Web Developer does not have any CSS formatting abilities. I looked into microsoft/vscode-css-languageservice#44 and it seems that CSS formatting extensions are created in the VSCode world so that the LS doesn't have to handle it. Also related: microsoft/vscode#39088

It seems that running something like https://prettier.io/ as a save or build action might work best (or just something that contributors can run before pushing a PR).

I could probably set up a package.json for the repo that contains prettier and a script to format. It does feel a bit weird to do this for a maven project, however... but I'm not really against trying it out, could be cool.

@AObuchow
Copy link
Owner Author

Prettier supports pre-commit hooks: see option 6.

Ideally, an eclipse plugin that integrates Prettier would be nice (however there’s some work involved). It would be useful for Wild Web Developer users however

AObuchow added a commit that referenced this issue Jun 28, 2020
Part of #88

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
@AObuchow
Copy link
Owner Author

I found out how to get prettier to auto format on save in Eclipse using a custom builder:
image

For some reason, the builder runs 3 times however ): Will have to see if there's any way to fix this

AObuchow added a commit that referenced this issue Jun 29, 2020
Part of #88

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
AObuchow added a commit that referenced this issue Jun 29, 2020
Part of #88

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
@AObuchow
Copy link
Owner Author

AObuchow commented Jun 29, 2020

So I added prettier to the master branch.
This allows contributors to:

  • Format using npm run format (npm install must be run first to download prettier)
  • Format using the Prettier-Format project builder (Eclipse specific, see previous comment's gif)
  • Format using a git pre-commit hook (this must be copied to the contributors .git/hooks/ directory of the repo)

Mention of these formatting options should be put in the PR submission guidelines.

I've also reformatted the entire project's CSS. Hopefully, I didn't break anything :D

AObuchow added a commit that referenced this issue Jun 29, 2020
Part of #88

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
@AObuchow
Copy link
Owner Author

It might be a bit redundant now that the project is using prettier, but this github workflow could be used to make sure all CSS used in valid when pushing to master: https://github.com/github/super-linter

@AObuchow AObuchow added the build label Jul 1, 2020
@AObuchow
Copy link
Owner Author

For now, formatting with prettier using the previously mentioned options is good enough so I’m going to close this.

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

No branches or pull requests

3 participants