[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Thans for the patch!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61850



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360883: [clang-tidy] Removed superfluous and slightly 
annoying newlines in run-clang… (authored by JonasToth, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61850?vs=199230=199800#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61850

Files:
  clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -170,10 +170,10 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -170,10 +170,10 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.

LGTM then, i can commit for you again?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61850



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Sven Panne via Phabricator via cfe-commits]

svenpanne added a comment.

I think `ninja clang-test-tools` is the right target here, and it is happy with 
the change, which is not very surprising: AFAICT, there is just a single, very 
simple check for `run-clang-tidy.py`. Anyway, my manual testing with and 
without `-quiet` shows no problems, either. Furthermore, we have a patched 
version with the above change in our CI for quite some time, and it works fine. 
And if I read the git history correctly, the output should be identical to the 
one before a fix for interleaved output was landed (that fix added the newlines 
somehow).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61850



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D61850#1500330 , @svenpanne wrote:

> In D61850#1500298 , @JonasToth wrote:
>
> > [...] please ensure that e.g. clang-analyzer-* output looks proper as well 
> > and `-quiet` does, too.
>
>
> OK, are there any (unit) tests which I should run? If yes, how exactly? I 
> have a successfully built a fork of https://github.com/llvm/llvm-project via 
> ninja locally, but the documentation for the various subprojects is still a 
> bit hard to find to me. Is there a ninja target for `clang-tidy`-related 
> tests? Any hints to get started would be highly appreciated.
>
> This and the other output-related patches are my first submissions here, and 
> they are intentionally simple to get the workflow right. My real intention is 
> trying to improve/fix the `readability-identifier-naming` check/fixer, which 
> still has a few issues.


There are testing targets for clang-tidy and clang-tooling, i believe `ninja 
check-clang-tools` is includes clang-tidy (maybe `ninja check-clang-tooling` 
instead).
If this script is actually covered by testing is not know to me, i believe not.
For testing just run it over a project (e.g. parts of LLVM) with this tool 
instead of e.g. your distribution version of it.

For general development: `clang-tools-extra/test/clang-tidy/*` has the 
test-cases for clang-tidy checks, which are real world code examples that cover 
all representative cases (in theory).

1. Write/adjust test
2. Do changes in code
3. Test with `ninja check-clang-tools` / `ninja check-clang-tooling`

That usually works. If you encounter any issues feel free to ask here/IRC/mail, 
we are happy to help out :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61850



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Sven Panne via Phabricator via cfe-commits]

svenpanne added a comment.

In D61850#1500298 , @JonasToth wrote:

> [...] please ensure that e.g. clang-analyzer-* output looks proper as well 
> and `-quiet` does, too.


OK, are there any (unit) tests which I should run? If yes, how exactly? I have 
a successfully built a fork of https://github.com/llvm/llvm-project via ninja 
locally, but the documentation for the various subprojects is still a bit hard 
to find to me. Is there a ninja target for `clang-tidy`-related tests? Any 
hints to get started would be highly appreciated.

This and the other output-related patches are my first submissions here, and 
they are intentionally simple to get the workflow right. My real intention is 
trying to improve/fix the `readability-identifier-naming` check/fixer, which 
still has a few issues.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61850



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I agree with the direction, please ensure that e.g. clang-analyzer-* output 
looks proper as well and `-quiet` does, too.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61850



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Sven Panne via Phabricator via cfe-commits]

svenpanne created this revision.
svenpanne added a reviewer: alexfh.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The output of clang-tidy itself already has enough newlines, so the resulting 
output is more in line with the usual compiler output.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61850

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -170,10 +170,10 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -170,10 +170,10 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits