carlosgalvezp added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/add_new_check.py:324-328
   doc_files = []
-  for subdir in list(filter(lambda s: not s.endswith('.rst') and not 
s.endswith('.py'),
-                     os.listdir(docs_dir))):
+  for subdir in list(filter(os.path.isdir, os.listdir(docs_dir))):
     for file in filter(lambda s: s.endswith('.rst'), 
os.listdir(os.path.join(docs_dir, subdir))):
       doc_files.append([subdir, file])
   doc_files.sort()
----------------
ccotter wrote:
> carlosgalvezp wrote:
> > It feels this whole code could be made much simpler, readable and safer by 
> > just doing:
> > 
> >   doc_files = glob.glob(os.path.join(docs_dir, "**", "*.rst"), 
> > recursive=True)
> I gave this a shot, but realized the the glob returned the relative path 
> `../docs/clang-tidy/checks/<subdir>/<check>.rst`, and the existing logic only 
> adds `[<subdir>, <check>.rst]` to the list. Python3.10's `glob.glob` has 
> root_dir which would do the trick for us, but I don't think we can rely on 
> Python3 yet? I think using glob might not improve the readability all that 
> much since I'd have to strip out the `docs_dir` subpath, but happy to give 
> that a go based on your feedback.
Good point, I was thinking of something along these lines:


```
from pathlib import Path

for doc_file in map(lambda s: Path(s), glob.glob(os.path.join(docs_dir, "*", 
"*.rst"))):
  doc_files.append([doc_file.parts[-1], doc_file.parts[-2])
```

The current patch is already a great cleanup so I'm fine with it as well :) 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140772

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

Reply via email to