Open Bug 1523073 Opened 7 years ago Updated 4 years ago

Get rid of `mach clang-format`s --path flag

Categories

(Developer Infrastructure :: Source Code Analysis, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: jwatt, Unassigned)

References

Details

I never want to run mach clang-format on the entire tree, and I assume most other people don't either. If people always specify a path, it just created friction to have to type out '-p' every time. Could we not just make all the arguments after last flag (if any) paths? For me personally, this is the behavior that I expect given that's how most command line tools that act on part of a filesystem work. I regularly forget I need the -p, then have to go back, edit the command and run it again.

Agreed
We should just trigger an error when no argument are provided

I fee the need to specify by having this we remove the default functionality where we perform clang-format on the diff

hmm, actually, I am incorrect, "mach clang-format" will run on the current diff.
Not on the whole tree.

Oh, did the behavior here change at some point?

Anyway, how about we get rid of -p and then process as follows:

No args: Format the current diff.

One or more path args: Format the current diff, but only for the specified files (and error if a specified path is not in the current diff?).

Add an --all-files or --working-dir flag or something like that to change the behavior from acting on the current diff to acting on the files in the working directory, limited to the given paths, if any.

Nope, the first version (from Anthony Jones I believe) was only running on diff. I added the support of -p.
See the doc:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style/Formatting_C++_Code_With_clang-format

No args: Format the current diff.
One or more path args: Format the current diff, but only for the specified files (and error if a specified path is not in the current diff?).

I am not convinced. I think it would not follow the principle of least astonishment.

To me, if you do:
./mach clang-format dom/
the expectation should be that it is the same as:
./mach clang-format -p dom/

Would it match your expectation?

For your last point, it should in a different bug I think

The priority flag is not set for this bug.
:sylvestre, could you have a look please?

Flags: needinfo?(sledru)
Severity: normal → enhancement
Flags: needinfo?(sledru)
Priority: -- → P3
See Also: → 1514279
Product: Firefox Build System → Developer Infrastructure
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.