Page MenuHomePhabricator

Max_S (Max Sagebaum)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 8 2021, 4:27 AM (9 w, 6 d)

Recent Activity

Apr 16 2021

Max_S updated the diff for D99503: [clang-format] Inconsistent behavior regarding line break before access modifier.

Sorry for the inconvenience. These were the errors, that inspired this patch.

Apr 16 2021, 12:41 AM · Restricted Project, Restricted Project

Apr 15 2021

Max_S added a comment to D99503: [clang-format] Inconsistent behavior regarding line break before access modifier.

Sorry forgot about that: Max Sagebaum <max.sagebaum@scicomp.uni-kl.de>

Apr 15 2021, 10:50 PM · Restricted Project, Restricted Project
Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

Thank you for landing it. The information should be ok. But anyway: Max Sagebaum <max.sagebaum@scicomp.uni-kl.de>

Apr 15 2021, 3:55 AM · Restricted Project, Restricted Project
Max_S added a comment to D99503: [clang-format] Inconsistent behavior regarding line break before access modifier.

I do not have the access rights to the llvm git repo. Can you please land it for me.

Apr 15 2021, 3:45 AM · Restricted Project, Restricted Project
Max_S updated the diff for D99503: [clang-format] Inconsistent behavior regarding line break before access modifier.

Added changelog entry.

Apr 15 2021, 3:44 AM · Restricted Project, Restricted Project
Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

Thank you for the answer. It did not know that I have to land it myself.

Apr 15 2021, 3:22 AM · Restricted Project, Restricted Project
Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

Just wanted to ask if there is something missing for the merge?

Apr 15 2021, 2:00 AM · Restricted Project, Restricted Project
Max_S updated the diff for D99503: [clang-format] Inconsistent behavior regarding line break before access modifier.

Moved the test to their own section and fixed the formatting issues.

Apr 15 2021, 1:52 AM · Restricted Project, Restricted Project

Mar 31 2021

Max_S updated the diff for D98237: [clang-format] Option for empty lines after an access modifier..

Thank you for the reviews.

Mar 31 2021, 1:43 PM · Restricted Project, Restricted Project

Mar 30 2021

Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

In the last update I fixed now everything and changed the tests.

Mar 30 2021, 9:44 AM · Restricted Project, Restricted Project
Max_S updated the diff for D98237: [clang-format] Option for empty lines after an access modifier..

Cleanup of tests and additonal logic for modifiers at the end of a class.

Mar 30 2021, 9:16 AM · Restricted Project, Restricted Project

Mar 29 2021

Max_S added a comment to D99503: [clang-format] Inconsistent behavior regarding line break before access modifier.

This is patch for a bug reported in 2019 https://bugs.llvm.org/show_bug.cgi?id=41870

Mar 29 2021, 6:33 AM · Restricted Project, Restricted Project
Max_S added a project to D99503: [clang-format] Inconsistent behavior regarding line break before access modifier: Restricted Project.
Mar 29 2021, 6:32 AM · Restricted Project, Restricted Project
Max_S requested review of D99503: [clang-format] Inconsistent behavior regarding line break before access modifier.
Mar 29 2021, 6:28 AM · Restricted Project, Restricted Project

Mar 23 2021

Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

If you follow people tweeting about clang-format (as I do) and you look through the bug tracking system, one major criticism of clang-format is that the second clang-format can be different from the first, sometimes an equilibrium can be found sometimes not.

When I started working on clang-format I was encouraged to use verifyFormat() as it tests that scenario and also tries to mess up the format and ensure it returns to the desired state. It found bugs in my code which would have made clang-format worse.

Apart from it being the convention I believe it makes for much more readable code, even if there is repetition as I don't need to keep cross referencing variables with rather obscure names NL_B_3_A_0_I_0 this is unnecessary noise and makes the code overly verbose.

No you'll need to check out what the messUp() function is actually doing but I think by and large IMHO we should stick with verifyFormat.

Mar 23 2021, 8:03 AM · Restricted Project, Restricted Project

Mar 22 2021

Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

The tests in line 9462 and 9466 require some additional implementation. EmptyLineAfterAccessModifier is adding the three lines since it is configured to do so, but EmptyLineBeforeAccessModifier would remove these lines. Since EmptyLineBeforeAccessModifier can only access the old file and not the new one. It does not know that three lines have been written and probably could not remove them.

I would like to implement it in such a way, that EmptyLineAfterAccessModifier looks ahead and skips its logic if the next token is a also an access modifier such that the logic of EmptyLineBeforeAccessModifier takes precedence. I could not find a way to get the next line. Is this possible somehow? Does there exist some helper functionality?

Mar 22 2021, 3:40 AM · Restricted Project, Restricted Project
Max_S updated the diff for D98237: [clang-format] Option for empty lines after an access modifier..

Fixed interaction between Before and After configurations options. Before handles the case
of two access modifiers specification following each other.

Mar 22 2021, 3:30 AM · Restricted Project, Restricted Project

Mar 19 2021

Max_S added inline comments to D98237: [clang-format] Option for empty lines after an access modifier..
Mar 19 2021, 3:43 AM · Restricted Project, Restricted Project
Max_S updated the diff for D98237: [clang-format] Option for empty lines after an access modifier..

I added more tests for the interaction between MaxEmptyLinesToKeep, EmptyLineBeforeAccessModifier and EmptyLineAfterAccessModifier. During these tests I recognized, that EmptyLineBeforeAccessModifier = Always,Leave will (should adhere) to MaxEmptyLinesToKeep. So the option does not force the new line it only applies it if it is missing. I changed the logic for EmptyLineAfterAccessModifier accordingly.

Mar 19 2021, 3:37 AM · Restricted Project, Restricted Project

Mar 15 2021

Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

As already described in the update I changed the option name and how it behaves. It is now more in line with EmptyLineBeforeAccessModifier.

Mar 15 2021, 3:11 AM · Restricted Project, Restricted Project
Max_S updated the diff for D98237: [clang-format] Option for empty lines after an access modifier..

Changed the option to EmptyLineAfterAccessModifier and allowed the options Never, Leave and Always. Updated the tests accordingly and also added an entry in the changelog.

Mar 15 2021, 3:00 AM · Restricted Project, Restricted Project

Mar 10 2021

Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

Just out of interest and we are supposed to ask for this but can you point to public style guide that uses this style. (actually I don't mind if other formatting tools have this capability and you highlight it, like astyle or editorConfig etc)

From my perspective this seems like a reasonable suggestion. (even if I'm unlikely to use it myself ;-))

Mar 10 2021, 5:52 AM · Restricted Project, Restricted Project
Max_S added inline comments to D98237: [clang-format] Option for empty lines after an access modifier..
Mar 10 2021, 2:11 AM · Restricted Project, Restricted Project
Max_S added a comment to D98237: [clang-format] Option for empty lines after an access modifier..

I would like additional tests:

  • With at least 2 empty lines
  • With 0-2 empty lines without verifyFormat to demonstrate what is kept, added, and removed.
Mar 10 2021, 2:01 AM · Restricted Project, Restricted Project
Max_S updated the diff for D98237: [clang-format] Option for empty lines after an access modifier..

Updating D98237: [clang-format] Option for empty lines after an access modifier.

Mar 10 2021, 1:58 AM · Restricted Project, Restricted Project

Mar 8 2021

Max_S requested review of D98237: [clang-format] Option for empty lines after an access modifier..
Mar 8 2021, 11:49 PM · Restricted Project, Restricted Project