Add the definition, documentation, and implementation for a new clang-format option.
This is for new issue https://github.com/llvm/llvm-project/issues/62092
Differential D125171
[clang-format] Add a new clang-format option AlwaysBreakBeforeFunctionParameters jrmolin on May 7 2022, 10:00 AM. Authored by
Details Add the definition, documentation, and implementation for a new clang-format option. This is for new issue https://github.com/llvm/llvm-project/issues/62092
Diff Detail
Event TimelineComment Actions Please reupload with the complete context. Please add tests.
Comment Actions I am doing this for my team, which writes the security endpoint for Elastic Defend. The code is currently private, though the binaries are free to download and run. The number of contributors is around 30, and the lines of code is in the 100Ks (around 500K). I have not found a way to accomplish what this does with the available options. I am adding tests and am happy to maintain this. It is a rather small addition, so it is quite simple to keep updated.
Comment Actions Unfortunately, we don't. I can create one in the likeness of the others, but I'm not sure where it will get published. I will talk to my managers about publishing a style doc.
Comment Actions I updated the code comment in Format.h and ran the docs generator. I didn't modify the Macros section, but that got updated when I ran the docs generator. We don't have a published style guide, unfortunately. I can work towards that, but I don't know how long that would take. My team doesn't like to agree on formatting changes. Hence this patch. We rolled this into the 3.9.0 release, and have been stuck with that ever since. Comment Actions I still don't know what Please write it out long form means for the unit test.
Comment Actions if you go ahead an rebase your should get rG7a5b95732ade: [clang-format] NFC Format.h and ClangFormatStyleOptions.rst are out of date that will remove the need for the Macro section in the rst Comment Actions So pull in that git hash and re-diff? I couldn't get arcanist installed cleanly (php issues), so I've been using git diff -U999999... to generate the patches. I just keep pulling down main before creating a patch.
Comment Actions pulled main, re-generated the docs, then re-generated the diff. Also added a parse test and collapsed some nested if conditions.
Comment Actions I think I have hit all the requests now. We're in the middle of building release candidates and testing, so management is taking longer and longer to get back to me about a style guide for my team. We added it, because it forces a consistent look across all function declarations. I don't know what the next steps are now. I'm stuck; you're stuck. :shrug:
Comment Actions I started a style guide at https://github.com/elastic/endpoint/blob/add_style_guide/style_guide.md, but it hasn't been approved or merged to main. It's at least a start... Comment Actions I don't like using additional variables in unit tests, if someone changes 25400 multiple tests could break, each test should be stand alone in my view
This comment was removed by MyDeveloperDay. Comment Actions Can you show your example from a implementation case and not just a declaration i.e. what happens with int function1(int param1) {}
Comment Actions
Comment Actions
Comment Actions is it possible to point to a github issue that this related to in the review summary, if not please can you make one so we can track the issue its trying to solve
Comment Actions changing the added option from a boolean to an enum that takes Leave, Always, and Never.
Comment Actions there were a couple of bugs in the last patch that I missed. format tests all pass again with this one. I am using "Leave" as a transparent value, so no line breaks are added and none are removed -- the penalty calculus drives. Just like it was before this patch. It's the best I can do for making users not have to care about this option.
Comment Actions @jrmolin is this option really for breaking before the first parameter? Or are you trying to have one parameter per line as shown by the examples in your GitHub issue? Also, how would it interact with BAS_AlwaysBreak, BinPackParameters, and AllowAllParametersOfDeclarationOnNextLine? What if there is only one parameter? And do we really want something like the following for example? template <typename T> void baz( T t) requires C<T>
|
Regererate