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 fish_indent --check #7251

Closed
IlanCosman opened this issue Aug 8, 2020 · 8 comments
Closed

Add fish_indent --check #7251

IlanCosman opened this issue Aug 8, 2020 · 8 comments

Comments

@IlanCosman
Copy link
Contributor

Similarly to many other formatters, fish_indent should have a -c/--check option that will tell the user if the fish file is formatted as fish_indent would like.

This is currently achievable with fish_indent $file | diff - $file, but I think it should be added as a standard option.

fish, version 3.1.2

Linux archlabs 5.7.12-arch1-1 #1 SMP PREEMPT Fri, 31 Jul 2020 17:38:22 +0000 x86_64 GNU/Linux

xterm-256color

Clean fish

@faho
Copy link
Member

faho commented Aug 8, 2020

I don't see the benefit? Why not just reformat?

@IlanCosman
Copy link
Contributor Author

Here's a popular formatter with a --check option and a bit of explanation around it:
https://prettier.io/docs/en/cli.html#--check

As they said above, I'm interested in using this in a CI pipeline. With linters and formatters as part of the pipeline, I'd prefer that the commit remains unchanged.

@faho
Copy link
Member

faho commented Aug 8, 2020

I mean you can also just use it in the pre-commit hook, or write that thing above.

The way fish_indent is written would make this quite difficult, because it doesn't actually look at the source. It looks at the parse tree.

@faho
Copy link
Member

faho commented Aug 8, 2020

Okay, no, this is much easier than I assumed.

@faho faho closed this as completed in 2cdd6df Aug 8, 2020
@faho faho added this to the fish 3.2.0 milestone Aug 8, 2020
@faho
Copy link
Member

faho commented Aug 8, 2020

Yeah, added.

However fish_indent had some rather annoying regressions since 3.1.2, see #7252.

@IlanCosman
Copy link
Contributor Author

Thanks for the quick response and feature addition @faho 😄

@IlanCosman
Copy link
Contributor Author

Hi @faho Sorry to bother you about this issue again😕, I really appreciate you adding it 😄.

I should have thought of this in the original issue:

fish_indent --check should list the file names that fail. This is important for making it pleasant to work with.

faho added a commit that referenced this issue Aug 10, 2020
Also return the number of failed files.

I decided to *just* print the filenames (newline-separated because
NULLs are annoying here) to make it easier to deal with.

See #7251.
@faho
Copy link
Member

faho commented Aug 10, 2020

Yeah, sure, done.

It now returns the number of failed files and prints newline-separated names of the failed files (this also means it doesn't quit at the first failure - which is usually a bad idea anyway).

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants