[PATCH] D53482: Add clang-format stability check with FormatTests

2019-11-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D53482#1737203 , @aardappel wrote: > @MyDeveloperDay thanks for your thoughts.. while `-stable` would be helpful > once you know you have the issue (or to permanently turn on in a git > pre-submit), it does not help

[PATCH] D53482: Add clang-format stability check with FormatTests

2019-11-07 Thread Wouter van Oortmerssen via Phabricator via cfe-commits
aardappel added a comment. @MyDeveloperDay thanks for your thoughts.. while `-stable` would be helpful once you know you have the issue (or to permanently turn on in a git pre-submit), it does not help developers that don't even know this is a thing, and waste a bunch of time figuring out what

[PATCH] D53482: Add clang-format stability check with FormatTests

2019-11-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. As @krasimir said we wouldn't normally commit tests which just break as they'd fail on the buildbot and someone would revert your change within 24 hours.. however, what you have here raises an interesting conversation This bug of instability is often raised,

[PATCH] D53482: Add clang-format stability check with FormatTests

2019-11-04 Thread Wouter van Oortmerssen via Phabricator via cfe-commits
aardappel added a comment. @krasimir first you say that fixing it is hard, then when someone wants to attempt, that it is not necessary? Plenty of people run `clang-format` in automated fashion as part of some pipeline, seems to me results ping-ponging between two formattings and cluttering

[PATCH] D53482: Add clang-format stability check with FormatTests

2018-10-25 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added a comment. The problem with this is that I believe fixing this issue is not worth it, hence this patch is unnecessary. Feel free to reach out to djasper@ for a second opinion. For completeness, was the unstability a problem just in FormatTest.cpp, how about the other

[PATCH] D53482: Add clang-format stability check with FormatTests

2018-10-23 Thread Vladimir Glavnyy via Phabricator via cfe-commits
vglavnyy added a comment. @krasimir thank you for review. The patch code has been updated. I hope this patch will help to start to fix this issue. Probably will be helpful to add an optional debug flag will enable a twice-run checking for every run of clang-format.

[PATCH] D53482: Add clang-format stability check with FormatTests

2018-10-23 Thread Vladimir Glavnyy via Phabricator via cfe-commits
vglavnyy updated this revision to Diff 170663. vglavnyy added a comment. A twice-format problem: the format of format isn't format. This commit surface an instability problem in clang-format at unit-tests level. The patch adds double-checking format method for all test and adds a stub for tests

[PATCH] D53482: Add clang-format stability check with FormatTests

2018-10-22 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir requested changes to this revision. krasimir added a comment. This revision now requires changes to proceed. Sadly I tried this out a year ago and hit the same thing. A root cause is that tabs have variable column length depending on their start column, and in clang-format tokens are

[PATCH] D53482: Add clang-format stability check with FormatTests

2018-10-21 Thread Vladimir Glavnyy via Phabricator via cfe-commits
vglavnyy created this revision. vglavnyy added reviewers: djasper, krasimir. Herald added a subscriber: cfe-commits. Twice running clang-format may give unstable result for some code samples, for example: https://bugs.llvm.org/show_bug.cgi?id=23728 This commit adds stability check to