enyquist added a comment.
Looks fine to me, although I confess I did not build and run it because I don't
have the time to set up the environment again, took a few hours last time I
built from scratch (side note, if there's an easy way to speed up llvm/clang
compilation up I'd love to hear it :
enyquist added a comment.
@smilewithani @lassi.niemisto @mewmew feel free to take a stab at it
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D28462/new/
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-
enyquist added a comment.
@klimek having gotten that out of the way, I do occasionally drink too much and
have sudden urges to re-implement things from scratch. Close it if you need to,
since I can't commit to anything, but it it happens to be still open on one
of those nights, who knows, m
enyquist added a comment.
@klimek fair point. To be honest, I've pretty much lost interest / momentum on
this feature, I very much doubt I will ever go back and re-implement from
scratch as you suggest.
Not meaning to sound rude, I just don't want to waste anyone's time who is
waiting for this
enyquist added a comment.
As far as I know, there are no updates required from me for this pull request--
I rebased on the main trunk recently, and will do it again tonight to be sure.
So it should be compiling/working just fine.
I believe it is just awaiting final approval from somebody.
Repo
enyquist added a comment.
@dtzWill thanks for the suggestion, I have submitted this change to the weekly
review corner.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
enyquist updated this revision to Diff 135744.
enyquist added a comment.
Rebased on current master. No functional changes, just a minor conflict where a
variable got re-named
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format
enyquist added a comment.
Gaaah. I'm so sorry. I wrote that last comment months ago and never submitted.
No wonder you guys weren't responding.
Comment at: lib/Format/WhitespaceManager.cpp:470
// definitions.
return C.Tok->is(TT_StartOfName) |
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:431
+
+ // Special case for AlignTokens: for all other alignment cases,
+ // the current sequence is ended when a comma or a scope change
djasper wrote:
> enyqu
enyquist updated this revision to Diff 96333.
enyquist marked an inline comment as done.
enyquist added a comment.
Addressed comments
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Fo
enyquist marked 2 inline comments as done.
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:413
+
+ while (Param && !Param->is(tok::l_paren)) {
+if (!Param->is(tok::identifier) && !Param->is(tok::comma))
djasper wrote:
> enyquist
enyquist updated this revision to Diff 95680.
enyquist added a comment.
Rebased on latest
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/Whit
enyquist updated this revision to Diff 93341.
enyquist added a comment.
Apologies-- forgot to update a comment
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.
enyquist updated this revision to Diff 9.
enyquist marked 2 inline comments as done.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/Whitesp
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:413
+
+ while (Param && !Param->is(tok::l_paren)) {
+if (!Param->is(tok::identifier) && !Param->is(tok::comma))
djasper wrote:
> enyquist wrote:
> > djasper wrote:
> > > I think you
enyquist updated this revision to Diff 93204.
enyquist marked 3 inline comments as done.
enyquist added a comment.
Addressed all comments, except for the one about FormatToken.MatchingParen (see
reply comment)
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/
enyquist marked 8 inline comments as done.
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:287
SmallVector
&Changes,
+bool ConsiderScope, bool ConsiderCommas,
unsig
enyquist updated this revision to Diff 93092.
enyquist added a comment.
Option implemented completely in WhitespaceManager.cpp, so no overhead if the
option isn't used. Also some minor style fixes.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
enyquist added a comment.
@sylvestre.ledru No, unfortunately not. My apologies, I've been taken up with
work mostly.
I will make a marked attempt to do it this weekend :)
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailin
enyquist added a comment.
Thanks :) I should get a chance to return to this next week.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-
enyquist added inline comments.
Comment at: Format/FormatToken.h:148
+ /// \brief Whether the token is the final token in the identifier of a PP
+ // macro. This will be either 1) the identifier token following the 'define'
djasper wrote:
> This adds a lot of
enyquist updated this revision to Diff 84478.
enyquist added a comment.
Forgot to set "AlignConsecutiveMacros = false" in getLLVMStyle()
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
ClangFormatStyleOptions.rst
Format/Format.cpp
Format/FormatTest.cpp
Format/FormatToken.h
enyquist added a comment.
Well, your patch is here for me to try, and it looks like it's been accepted.
So I guess I should just pull my finger out and try it :)
Thanks for your response-- I'll let you know if I come across any issues.
https://reviews.llvm.org/D21279
enyquist added a comment.
Hey bmharper :) I've got a review open that conflicts with this one, just
having a look to see what I'll need to refactor
(https://reviews.llvm.org/D28462).
In fact, I have a question-- the conflict is specifically in
WhitespaceManager.cpp. Since I needed to detect PP
enyquist created this revision.
enyquist added reviewers: djasper, sylvestre.ledru.
enyquist added a subscriber: cfe-commits.
enyquist set the repository for this revision to rL LLVM.
enyquist added a project: clang-c.
Herald added a subscriber: klimek.
This option behaves similarly to AlignConsec
25 matches
Mail list logo
