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() ---------------- carlosgalvezp wrote: > 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 :) > > Sorry, I meant: ``` doc_files.append([doc_file.parts[-2], doc_file.parts[-1]) ``` 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