HazardyKnusperkeks marked 2 inline comments as done.
HazardyKnusperkeks added a comment.

In D92257#2422381 <https://reviews.llvm.org/D92257#2422381>, @MyDeveloperDay 
wrote:

> I think fundamentally from my perspective this seem ok, out of interest can I 
> ask what drove you to require it?
>
> My assumption is that some people write comments like
>
>   // Free comment without space
>
> and you want to be able to consistently format it to be (N spaces, as 
> clang-format already does 1 space correct?)
>
>   //  Free comment without space
>
> is that correct? is there a common style guide asking for that? what is the 
> rationale

I will go for `{0,0}`, so no space between `//` and the text, I don't know 
about a style guide asking for it, other than my own. (Which I can dictate in 
my company.) :)
I have just recently started using clang-format and it does not everything the 
the way I want to, on some aspects I have adapted, but on others I try to "fix" 
it, you can expect some more changes from me in the next time.



================
Comment at: clang/lib/Format/BreakableToken.cpp:790
+          (Style.Language != FormatStyle::LK_TextProto ||
+           OriginalPrefix[i].substr(0, 2) != "##")) {
+        Prefix[i] = IndentPrefix.str();
----------------
MyDeveloperDay wrote:
> is this case covered by a unit test at all? sorry can you explain why you are 
> looking for "##"?
It is covered by multiple tests, that's how I was made aware of it. :)
If you look at the code before it only adds a space if the old prefix is "#" 
not "##" which is also found by `getLineCommentIndentPrefix`. As it seems in 
`TextProto` "##" should not be touched. I can of course add a test in my test 
function.

Now I see a change, in the code before "#" was only accepted when the language 
is `TextProto`, now it is always. But I think for that to happen the parser (or 
lexer?) should have assigned something starting with"#" as comment, right? But 
I can change that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92257/new/

https://reviews.llvm.org/D92257

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to