[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan added a comment. @DmitryPolukhin Sorry, I didn't have time recently. Thanks a lot for taking care! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 ___ cfe-commits mailin

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

DmitryPolukhin added a comment. Sent D80301 for review. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 ___ cfe-commits mailing list cfe-commi

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

DmitryPolukhin added a comment. @mgehre @yvvan it seems that the issue still not fixed and '\n' gets duplicated in the replacements. Are you going to fix this issue or I should create a patch to fix it? Before this change '\n' was actually processed correctly `ReplacementText: '#include \n\n'`

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan added a comment. @mgehre I think we need to adjust `denormalize(const IO &)` method here to convert \n back properly. It seems I missed it in my patch. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 _

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

mgehre added a comment. Could be - did it handle it correctly for your include case here? And if this is a general yaml string thing, shouldn't the replacement of newlines (for both ways) happen in the yaml parser/writer instead of clang/Tooling? Repository: rL LLVM CHANGES SINCE LAST ACTI

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan added a comment. @mgehre From your comment it seems that `clang-apply-replacements` handles the YAML wrong and does not make the proper conversion back from "\n\n" to "\n" Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

mgehre added a comment. This change breaks `modernize-use-using` fixits. The code `typedef int A, B;` is replaced into using A = int; using B = int; when directly applying fixits with clang-tidy. But it is replaced into using A = int; using B = int; when applying fixits with `clang-

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

This revision was automatically updated to reflect the committed changes. Closed by commit rL365017: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value (authored by yvvan, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior t

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan added a comment. ok, will do it through svn. i forgot that clang repo is called "cfe" so it's there CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan added a comment. This script does not seem to work properly on windows. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr added a comment. SVN repo is still there. However, I don't know how to commit using SVN, I commit using the git-svn integration (which still commits using the "svn" command under the hood, but as a user you will be interacting with a git repo). git clone https://github.com/llvm/llv

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

nik added a comment. In D63482#1567897 , @yvvan wrote: > I have a commit access but I don't understand how am I supposed to commit > (haven't done that for a while). There's no clang svn repo anymore. Do you > know what's the current state of repositorie

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

nik added a comment. In D63482#1567927 , @nik wrote: > In D63482#1567897 , @yvvan wrote: > > > I have a commit access but I don't understand how am I supposed to commit > > (haven't done that for a while). There's

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan added a comment. I have a commit access but I don't understand how am I supposed to commit (haven't done that for a while). There's no clang svn repo anymore. Do you know what's the current state of repositories and where to commit? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks! Do you have commit access? Would you like me to commit this patch for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 _

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan updated this revision to Diff 207483. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 Files: clang/include/clang/Tooling/ReplacementsYaml.h clang/unittests/Tooling/ReplacementsYamlTest.cpp Index: clang/unittests/Tooling/ReplacementsYam

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan marked an inline comment as done. yvvan added inline comments. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:35 +: FilePath(""), Offset(0), Length(0), ReplacementText("") { + size_t lineBreakPos = ReplacementText.find('\n'); + while (lineBreak

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:35 +: FilePath(""), Offset(0), Length(0), ReplacementText("") { + size_t lineBreakPos = ReplacementText.find('\n'); + while (lineBreakPos != std::string::npos) {

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan marked an inline comment as done. yvvan added inline comments. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:35 +: FilePath(""), Offset(0), Length(0), ReplacementText("") { + size_t lineBreakPos = ReplacementText.find('\n'); + while (lineBreak

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:35 +: FilePath(""), Offset(0), Length(0), ReplacementText("") { + size_t lineBreakPos = ReplacementText.find('\n'); + while (lineBreakPos != std::string::npos) {

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan updated this revision to Diff 207062. yvvan added a comment. Sorry for delay. Test added, redundant comments removed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 Files: clang/include/clang/Tooling/ReplacementsYaml.h clang/unittests

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr added a comment. Thanks! Please add tests in `./unittests/Tooling/ReplacementsYamlTest.cpp`. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:43 +ReplacementText.replace(lineBreakPos, 1, "\n\n"); +// Get the next occurrence from the current p

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

yvvan created this revision. yvvan added reviewers: gribozavr, nik. Herald added a subscriber: xazax.hun. Currently this check generates the replacement with the newline in the end. The proper way to export it to YAML is to have two \n\n instead of one. Without this fix clients should reinterpre