[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

In D120331#3384889 , @SAtacker wrote:

> Gentle reminder to commit on my behalf
> @JonasToth
> @alexfh   
> @aaron.ballman
> @njames93 
> @LegalizeAdulthood
>
> Thank you

Thanks for the reminder -- I've commit on your behalf in 
9e1f4f13984186984ba37513372c1b7e0c4ba4fd 
, thank 
you for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Shreyas via Phabricator via cfe-commits]

SAtacker marked an inline comment as done.
SAtacker added a comment.

Gentle reminder
@JonasToth  
@alexfh 
@aaron.ballman  
@njames93   
@LegalizeAdulthood


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Shreyas via Phabricator via cfe-commits]

SAtacker marked an inline comment as done.
SAtacker added a comment.

Gentle ping @alexfh @JonasToth 
Thank you.




Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:228
   'default')
-  parser.add_argument('-config', default=None,
+  group = parser.add_mutually_exclusive_group()
+  group.add_argument('-config', default=None,

JonasToth wrote:
> nit: i think `config_group` or so would be better to show that its only 
> config-related. This name is too generic i feel.
Thanks, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Shreyas via Phabricator via cfe-commits]

SAtacker updated this revision to Diff 412397.
SAtacker added a comment.

[clang-tidy][run-clang-tidy.py] Rename `group` to `config_group`

Signed-off-by: Shreyas Atre 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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
@@ -96,8 +96,8 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config,
-line_filter, use_color):
+extra_arg, extra_arg_before, quiet, config_path,
+config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -127,7 +127,9 @@
   start.append('-p=' + build_path)
   if quiet:
   start.append('-quiet')
-  if config:
+  if config_path:
+  start.append('--config-file=' + config_path)
+  elif config:
   start.append('-config=' + config)
   start.append(f)
   return start
@@ -188,8 +190,8 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter,
- args.use_color)
+ args.quiet, args.config_path, args.config,
+ args.line_filter, args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
 output, err = proc.communicate()
@@ -223,7 +225,8 @@
   parser.add_argument('-checks', default=None,
   help='checks filter, when not specified, use clang-tidy '
   'default')
-  parser.add_argument('-config', default=None,
+  config_group = parser.add_mutually_exclusive_group()
+  config_group.add_argument('-config', default=None,
   help='Specifies a configuration in YAML/JSON format: '
   '  -config="{Checks: \'*\', '
   '   CheckOptions: [{key: x, '
@@ -231,6 +234,12 @@
   'When the value is empty, clang-tidy will '
   'attempt to find a file named .clang-tidy for '
   'each source file in its parent directories.')
+  config_group.add_argument('-config-file', default=None,
+  help='Specify the path of .clang-tidy or custom config '
+  'file: e.g. -config-file=/some/path/myTidyConfigFile. '
+  'This option internally works exactly the same way as '
+  '-config option after reading specified config file. '
+  'Use either -config-file or -config, not both.')
   parser.add_argument('-header-filter', default=None,
   help='regular expression matching the names of the '
   'headers to output diagnostics from. Diagnostics from '
@@ -283,8 +292,8 @@
  None, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter,
- args.use_color)
+ args.quiet, args.config_path, args.config,
+ args.line_filter, args.use_color)
 invocation.append('-list-checks')
 invocation.append('-')
 if args.quiet:


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
@@ -96,8 +96,8 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config,
-line_filter, use_color):
+extra_arg, extra_arg_before, quiet, config_path,
+config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -127,7 +127,9 @@
   start.append('-p=' + build_path)
   if quiet:
   

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Shreyas via Phabricator via cfe-commits]

SAtacker added a comment.

In D120331#3354021 , @JonasToth wrote:

> LGTM, but please adjust the naming nit to a better name.

Thanks for the review again. 
Will rename the `group` to `config_group` as you suggested and I feel that too.

> do you have commit rights? Otherwise someone (e.g. me) could commit on your 
> behalf, of course with a remark that its your contribution :)
>
> (btw. go ahead and ping me again, I am busy with too many things right now :/)

I don't have the access to land this commit into the llvm-project. Until then I 
shall trust this wonderful process :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Jonas Toth via Phabricator via cfe-commits]

JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.
Herald added a project: All.

LGTM, but please adjust the naming nit to a better name.

do you have commit rights? Otherwise someone (e.g. me) could commit on your 
behalf, of course with a remark that its your contribution :)




Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:228
   'default')
-  parser.add_argument('-config', default=None,
+  group = parser.add_mutually_exclusive_group()
+  group.add_argument('-config', default=None,

nit: i think `config_group` or so would be better to show that its only 
config-related. This name is too generic i feel.



Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:236
   'each source file in its parent directories.')
+  parser.add_argument('-config-file', default=None,
+  help='Specify the path of .clang-tidy or custom config'

SAtacker wrote:
> JonasToth wrote:
> > please ensure that those option exclude each other. right now it could be 
> > used with both options which is a contradiction for `clang-tidy`
> Do you mean mutual exclusion? From the docs: 
> https://docs.python.org/3/library/argparse.html#mutual-exclusion
Yes, thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Shreyas via Phabricator via cfe-commits]

SAtacker added a comment.

@JonasToth Could you please review it?




Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:236
   'each source file in its parent directories.')
+  parser.add_argument('-config-file', default=None,
+  help='Specify the path of .clang-tidy or custom config'

JonasToth wrote:
> please ensure that those option exclude each other. right now it could be 
> used with both options which is a contradiction for `clang-tidy`
Do you mean mutual exclusion? From the docs: 
https://docs.python.org/3/library/argparse.html#mutual-exclusion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Shreyas via Phabricator via cfe-commits]

SAtacker updated this revision to Diff 410736.
SAtacker marked an inline comment as done.
SAtacker added a comment.

Squash commits D120385 , D120387 
 into D120331 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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
@@ -96,8 +96,8 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config,
-line_filter, use_color):
+extra_arg, extra_arg_before, quiet, config_path,
+config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -127,7 +127,9 @@
   start.append('-p=' + build_path)
   if quiet:
   start.append('-quiet')
-  if config:
+  if config_path:
+  start.append('--config-file=' + config_path)
+  elif config:
   start.append('-config=' + config)
   start.append(f)
   return start
@@ -188,8 +190,8 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter,
- args.use_color)
+ args.quiet, args.config_path, args.config,
+ args.line_filter, args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
 output, err = proc.communicate()
@@ -223,7 +225,8 @@
   parser.add_argument('-checks', default=None,
   help='checks filter, when not specified, use clang-tidy '
   'default')
-  parser.add_argument('-config', default=None,
+  group = parser.add_mutually_exclusive_group()
+  group.add_argument('-config', default=None,
   help='Specifies a configuration in YAML/JSON format: '
   '  -config="{Checks: \'*\', '
   '   CheckOptions: [{key: x, '
@@ -231,6 +234,12 @@
   'When the value is empty, clang-tidy will '
   'attempt to find a file named .clang-tidy for '
   'each source file in its parent directories.')
+  group.add_argument('-config-file', default=None,
+  help='Specify the path of .clang-tidy or custom config '
+  'file: e.g. -config-file=/some/path/myTidyConfigFile. '
+  'This option internally works exactly the same way as '
+  '-config option after reading specified config file. '
+  'Use either -config-file or -config, not both.')
   parser.add_argument('-header-filter', default=None,
   help='regular expression matching the names of the '
   'headers to output diagnostics from. Diagnostics from '
@@ -283,8 +292,8 @@
  None, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter,
- args.use_color)
+ args.quiet, args.config_path, args.config,
+ args.line_filter, args.use_color)
 invocation.append('-list-checks')
 invocation.append('-')
 if args.quiet:


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
@@ -96,8 +96,8 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config,
-line_filter, use_color):
+extra_arg, extra_arg_before, quiet, config_path,
+config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -127,7 

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:236
   'each source file in its parent directories.')
+  parser.add_argument('-config-file', default=None,
+  help='Specify the path of .clang-tidy or custom config'

please ensure that those option exclude each other. right now it could be used 
with both options which is a contradiction for `clang-tidy`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120331

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

[PATCH] D120331: [clang-tidy][run-clang-tidy.py] Add --config-file= option

[Shreyas via Phabricator via cfe-commits]

SAtacker created this revision.
SAtacker added a reviewer: JonasToth.
Herald added subscribers: carlosgalvezp, xazax.hun.
SAtacker requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

- Link to the GitHub Issue: https://github.com/llvm/llvm-project/issues/53745

Details:

- Added config_path variable within the python script which makes the required 
call to the clang-tidy binary with --config-file option.
- If the config_path is None then config will be used.
- No error is raised if both are given but silently chooses config_path over 
config

Signed-off-by: Shreyas Atre 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120331

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
@@ -96,8 +96,8 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config,
-line_filter, use_color):
+extra_arg, extra_arg_before, quiet, config_path, 
+config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -127,7 +127,9 @@
   start.append('-p=' + build_path)
   if quiet:
   start.append('-quiet')
-  if config:
+  if config_path:
+  start.append('--config-file=' + config_path)
+  elif config:
   start.append('-config=' + config)
   start.append(f)
   return start
@@ -188,8 +190,8 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter,
- args.use_color)
+ args.quiet, args.config_path, args.config,
+ args.line_filter, args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
 output, err = proc.communicate()
@@ -231,6 +233,12 @@
   'When the value is empty, clang-tidy will '
   'attempt to find a file named .clang-tidy for '
   'each source file in its parent directories.')
+  parser.add_argument('-config-file', default=None,
+  help='Specify the path of .clang-tidy or custom config'
+  'file: e.g. -config-file=/some/path/myTidyConfigFile'
+  'This option internally works exactly the same way as'
+  '-config option after reading specified config file.'
+  'Use either -config-file or -config, not both.')
   parser.add_argument('-header-filter', default=None,
   help='regular expression matching the names of the '
   'headers to output diagnostics from. Diagnostics from '
@@ -283,8 +291,8 @@
  None, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter,
- args.use_color)
+ args.quiet, args.config_path, 
args.config, 
+ args.line_filter, args.use_color)
 invocation.append('-list-checks')
 invocation.append('-')
 if args.quiet:


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
@@ -96,8 +96,8 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config,
-line_filter, use_color):
+extra_arg, extra_arg_before, quiet, config_path, 
+config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
@@ -127,7 +127,9 @@
   start.append('-p=' + build_path)
   if quiet:
   start.append('-quiet')
-  if config:
+  if config_path:
+  start.append('--config-file=' + config_path)
+  elif config:
   start.append('-config='