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

Fails to generate compatible .clang-format file #1

Closed andrei-ng closed 2 years ago

andrei-ng commented 2 years ago

I tried running it on your example in standalone/main.cpp. I tried various versions of clan-format, up to version 12. None seem to work.

Which clang-version did you use ?

I can provide a full log, but for a sample, see below

SpaceInEmptyParentheses: true                      # parameter failed
SpacesBeforeTrailingComments: 0                    # parameter failed
SpacesInAngles: Never                              # parameter failed
SpacesInCStyleCastParentheses: true                # parameter failed
SpacesInConditionalStatement: true                 # parameter failed
SpacesInContainerLiterals: true                    # parameter failed
SpacesInParentheses: true                          # parameter failed
Standard: c++03                                    # parameter failed
TabWidth: 0                                        # parameter failed
UseCRLF: true                                      # parameter failed
# Total evaluation time: 17s:637ms:47µs
# Average evaluation time: 0s:41ms:992µs per parameter value
# Estimated time left: 0s:209ms:964µs
==============================

Adjust UseTab
Evaluating UseTab: UT_Never (failed option)
Evaluating UseTab: UT_ForIndentation (failed option)
Evaluating UseTab: UT_ForContinuationAndIndentation (failed option)
Evaluating UseTab: UT_AlignWithSpaces (failed option)
Evaluating UseTab: UT_Always (failed option)
Parameter UseTab did not affect the output

Inheriting innocuous entry AccessModifierOffset
Inheriting failed entry AlignArrayOfStructures
Inheriting failed entry AlignConsecutiveAssignments
Inheriting failed entry AlignConsecutiveBitFields
Inheriting failed entry AlignConsecutiveDeclarations
Inheriting failed entry AlignConsecutiveMacros
alandefreitas commented 2 years ago

Hi @andrei-ng

Thanks for trying this. It's good to see it works for other people. You might be the second user of this code. :D

The background is this was tested with clang-format 13 initially because it was the most recent version I found at the time, and installing clang-format 14 was a little too painful. The intention was to get started on another project where people don't use clang-format but I still needed to configure my IDE to something. I tried some similar projects I found on github in Python but it just took forever and never arrived at anything useful. This tool generated a good starting point although we have some formatting rules where clang-format cannot help us.

The way I generalized that to earlier versions of clang-format was by showing a failed option when that version of clang-format does not support the feature. This means the program is still working and running. It's just ignoring that option.

Of course, there's not much we can do about a feature an earlier version of clang-format doesn't support. What we can do is improve the messages or something.

On a side note, you will notice all of these tools have a problem with loose options that begin with "Allow". Allowing something almost always has a smaller distance to the original code than not allowing it. Even when it's not smaller, it's probably related to some other option. What we would need to make this work consistently is some kind of threshold on the cost/benefit of allowing it. I don't think any of these tools implement that yet.

andrei-ng commented 2 years ago

Hi @alandefreitas ,

Thank you for the explanation and the hints. Now more things make sense indeed.

I did not notice the warning concerning the version of clang, as probably the warning message was at the top of the stdout once the program started running. My suggestion would be to mention this in the README. Alternatively, you could prompt the user with a choice to proceed [y/N] if clang version is lower than 13, etc ...

I did find the failed option a bit misleading. Typically failure implies unable to proceed. Maybe renaming it as deprecated option or option ignored/skipped would be a bit clearer. I do agree that removing the message would not be ideal.

Thanks for your effort into making this tool available. It did give me a good starting point for a .clang-format file for my project.

I will close this issue as it is clear that it works OK with clang-format 13

alandefreitas commented 2 years ago

I did find the failed option a bit misleading. Typically failure implies unable to proceed. Maybe renaming it as deprecated option or option ignored/skipped would be a bit clearer. I do agree that removing the message would not be ideal.

I've improved that message a little. :)

This should work with any version of clang-format now. The options that are unavailable are commented out in the final result.