alandefreitas / clang-unformat

A simple tool to infer a .clang-format file from existing code
Boost Software License 1.0
58 stars 13 forks source link

The "output" argument is not copied from vm #9

Closed zeule closed 1 year ago

zeule commented 1 year ago

... in parse_cli() and ignored?

alandefreitas commented 1 year ago

What?

zeule commented 1 year ago

https://github.com/alandefreitas/clang-unformat/blob/master/standalone/cli_config.cpp#L63 you do not set c.output.

alandefreitas commented 1 year ago

Oh... what a terrible option name :laughing:

https://github.com/alandefreitas/clang-unformat/blob/a53f0a64d3e34c5f42fc0942f16623a15367eba1/standalone/cli_config.cpp#L48

You're correct. I think everyone has always used the default output location

https://github.com/alandefreitas/clang-unformat/blob/a53f0a64d3e34c5f42fc0942f16623a15367eba1/standalone/cli_config.cpp#L137

so we just never noticed the problem.

This fix is trivial:

https://github.com/alandefreitas/clang-unformat/blob/a53f0a64d3e34c5f42fc0942f16623a15367eba1/standalone/cli_config.cpp#L63

should be followed by c.output = vm["output"].as<fs::path>();.

We can also check if explicitly checking the output can cause any problems in

https://github.com/alandefreitas/clang-unformat/blob/a53f0a64d3e34c5f42fc0942f16623a15367eba1/standalone/cli_config.cpp#L133

but I just went through that and I don't think so.

zeule commented 1 year ago

I made the same trivial change earlier today, and it works for me as well.

alandefreitas commented 1 year ago

Since you caught the problem, do you want to open the PR?

zeule commented 1 year ago

Not really, thank you.

zeule commented 1 year ago

Thanks!