russellmcc added a comment.
Still waiting for feedback on this change! We've been happily using this patch
at my company for some time.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
___
cfe-commi
russellmcc added a comment.
Hey, still motivated to land this, but definitely don't want to break any
existing workflows.
In particular,
> but this afaict (unless I'm missing something :) will also affect the
> workflow where the provided range is 0-length range
I'm curious how to do this - a
russellmcc added a comment.
Sorry, coming back to this - line-filters are _inclusive_, so how do you
indicate a 0-length range from the command line?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
russellmcc added a comment.
Thanks for the explanation! I do understand your philosophy on this, and agree
with the desired behavior case you brought up where you have put in new braces.
After thinking about this more, the thing I really care about is that
clang-format is idempotent with a lin
russellmcc added a comment.
klimek: I'm sorry, I don't fully understand your proposed fix. Could you
explain it in more detail?
Without being able to respond to your proposal in detail, I strongly believe if
you pass in a line range to clang-format, it should not change lines outside
that ran
russellmcc added a comment.
@MyDeveloperDay Thanks for the approve! Yes, this patch has been working for
us as we've been using it on an internal fork since I opened the patch. As I
mentioned, without this patch clang-format is useless for us in CI.
CHANGES SINCE LAST ACTION
https://review
russellmcc added a comment.
@MyDeveloperDay Thanks for the approve! I'm not sure what the lingering
concerns are, as far as I know there have been no concerns since october of
last year.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
_
russellmcc added a comment.
@djasper Do you have any further concerns about this patch?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:/
russellmcc added a comment.
Yes, I don’t have commit access and would need someone else to land the patch.
Thanks!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-c
russellmcc added a comment.
Sorry for the ambiguity. I meant, "running clang-format on the changed lines
locally was not enough". I'll edit the description to clarify.
To go in more detail, let's imagine you changed line 3. You run clang-format
locally on line 3, but because of the bug fixed
russellmcc added a comment.
Ping! Still looking for help on this - I definitely don't want to diminish the
complexity of this code, and would really appreciate any help getting this in.
I've already apologized for the gap from feedback in July 2018 to response in
October - and I'm happy to ag
russellmcc added a comment.
Bump! Still waiting feedback on this.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-b
russellmcc added a comment.
Bump! Still looking for help here - thanks for the consideration.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
russellmcc added a comment.
Bump! Would really appreciate any feedback on this!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.
russellmcc added a comment.
Herald added a subscriber: jdoerfert.
Bump!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/
russellmcc added a comment.
Bump! Thanks for the consideration! Again, this is preventing us from rolling
out automated style checking on diffs at work.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
russellmcc added a comment.
Bump! Still watching this space - thanks so much for your time!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ht
russellmcc added a comment.
Bump! Thanks for the consideration
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/
russellmcc added a comment.
Bump! Thanks for the consideration.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin
russellmcc added a comment.
Thanks for the feedback! Does this mean that this won't be accepted? In my
opinion, without these extra options,
`AllowAllParametersOfDeclarationOnNextLine` is a very strange option. I don't
think I'm the only one who feels this way, based on the stack overflow
q
russellmcc added a comment.
Thanks for the feedback! This actually isn't a new formatting option, rather
it's fixing a bug where clang-format would change lines outside of the line
range asked for by the user. This was preventing us from using clang-format in
an automated setting.
CHANGES S
russellmcc added a comment.
Ping! Thanks for your consideration. I'm still quite motivated to land this,
please let me know if there's anything I can do, or if it's an unwanted patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
russellmcc marked 2 inline comments as done.
russellmcc added a comment.
Thanks for the feedback!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
___
cfe-commits mailing list
cfe-commits@lists.llvm.o
russellmcc updated this revision to Diff 181563.
russellmcc added a comment.
Respond to feedback
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
Files:
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineFormatter.h
lib/Format/Wh
russellmcc added a comment.
Bump! Thanks for your consideration.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin
russellmcc added a comment.
Bump! Thanks again for your time. As far as I can tell, it's ready for
another round of review!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
___
cfe-commits mailing
russellmcc added a comment.
Bump! Thanks again for your consideration
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D40988/new/
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
russellmcc updated this revision to Diff 175185.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54881/new/
https://reviews.llvm.org/D54881
Files:
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineFormatter.h
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager
russellmcc created this revision.
russellmcc added a reviewer: djasper.
Herald added a subscriber: cfe-commits.
When clang-format is set to always use tabs (with tab width 4), when asked to
format line 3 (using the `-lines=3:3` flag):
int f() {
int a;
foobar();
int b;
}
We
russellmcc added a comment.
Bump! Thanks again for your consideration.
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
bump! Thanks for your consideration.
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
Bump! Still looking for feedback on the latest round of changes.
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
Bump! Thanks for your time
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
Bump! Thanks for your time!
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
Sorry for dropping this for so long! Stuff got busy at work and I've been
happily using my fork with this change for some time. I would really like this
to get in, and promise to be responsive to feedback.
Comment at: lib/Format/ContinuationInden
russellmcc updated this revision to Diff 169288.
russellmcc added a comment.
Added suggested for loops to the test
https://reviews.llvm.org/D40988
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
unittests/
russellmcc added a comment.
Bump!
Thanks again for your time.
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
Bump! Thanks again for your feedback.
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:760
(!Style.AllowAllParametersOfDeclarationOnNextLine &&
State.Line->MustBeDeclaration) ||
+(!Style.AllowAllArgumentsOnNextLine &&
djasper wrote:
> This s
russellmcc added a comment.
Bump! Thanks for your consideration
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc marked an inline comment as not done.
russellmcc added a comment.
Okay; I think I've responded to all feedback at this point. Thanks for your
patience guiding me through my first contribution to this project. Let me know
what else I can do to help get this merged!
https://reviews.
russellmcc updated this revision to Diff 144280.
russellmcc marked an inline comment as done.
russellmcc added a comment.
Further update doc comments, render rst file
https://reviews.llvm.org/D40988
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Continuat
russellmcc updated this revision to Diff 144269.
russellmcc added a comment.
Responded to review feedback.
Improved documentation comments for new options.
While adding tests for the other constructor line break styles, I noticed an
inconsistency with the way "after colon" behaved with
`Constr
russellmcc added a comment.
Bump again! Any feedback would be quite appreciated.
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
Bump!
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
Bumping again - thanks so much for your time!
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc added a comment.
Ping! I believe all feedback has been addressed - further consideration would
be much appreciated.
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
russellmcc added a comment.
Thanks for the feedback! I'm very motivated to get some support for these
features since they are required for my style guide. Let me know if my recent
changes made sense to you.
Comment at: lib/Format/ContinuationIndenter.cpp:756
State
russellmcc updated this revision to Diff 133280.
russellmcc added a comment.
Responded to review feedback:
- Fix bug where `AllowAllArgumentsOnNextLine` overrode
`AllowAllParametersOfDeclarationOnNextLine`
- Add tests demonstrating independence of `AllowAllArgumentsOnNextLine` and
`AllowAllPara
russellmcc added a comment.
ping! Thanks for your consideration
Repository:
rC Clang
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
russellmcc created this revision.
Herald added a subscriber: klimek.
Add two new options,
AllowAllArgumentsOnNextLine and
AllowAllConstructorInitializersOnNextLine. These mirror the existing
AllowAllParametersOfDeclarationOnNextLine and allow me to support an
internal style guide where I work. I
51 matches
Mail list logo