| Message ID | 20250310125240.849705-1-roid@nvidia.com |
|---|---|
| State | Changes Requested |
| Delegated to: | aaron conole |
| Headers | show |
| Series | [ovs-dev,v2] checkpatch.py: Load multiple codespell dictionaries. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/cirrus-robot | success | cirrus build: passed |
Roi Dayan via dev <ovs-dev@openvswitch.org> writes: > Load dictionary_code.txt in addition to the default dictionary. > Add a new command line argument --dictionaries to be able > to specify codespell dictionaries. > > Signed-off-by: Roi Dayan <roid@nvidia.com> > Acked-by: Salem Sol <salems@nvidia.com> > --- Thanks for the update, it's close. I got a bit confused when trying to test with this - it seems the order of arguments is dependent: -S -D dictionary.txt -D dictionary_rare.txt will not load the new dictionaries, and we have to add -S at the end. BUT, I think the following incremental could work to resolve that. WDYT? Otherwise, we'll need to document that spellchecking needs to be after the dictionaries are specified. diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index dd68e307a5..6a456931a7 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -40,11 +40,9 @@ missing_authors = [] codespell_dictionaries = ['dictionary.txt', 'dictionary_code.txt'] __codespell_dict_reset_once = True +codespell_files = [] -def open_spell_check_dict(): - import enchant - - codespell_files = [] +def load_codespell_files(): try: import codespell_lib codespell_dir = os.path.dirname(codespell_lib.__file__) @@ -55,6 +53,22 @@ def open_spell_check_dict(): except: pass + +def load_codespell_words(): + global spell_check_dict, codespell_files + + for fn in codespell_files: + with open(fn) as f: + for line in f.readlines(): + words = line.strip().split('>')[1].strip(', ').split(',') + for word in words: + if spell_check_dict: + spell_check_dict.add_to_session(word.strip()) + + +def open_spell_check_dict(): + import enchant + try: extra_keywords = ['ovs', 'vswitch', 'vswitchd', 'ovs-vswitchd', 'netdev', 'selinux', 'ovs-ctl', 'dpctl', 'ofctl', @@ -125,13 +139,6 @@ def open_spell_check_dict(): spell_check_dict = enchant.Dict("en_US") - for fn in codespell_files: - with open(fn) as f: - for line in f.readlines(): - words = line.strip().split('>')[1].strip(', ').split(',') - for word in words: - spell_check_dict.add_to_session(word.strip()) - for kw in extra_keywords: spell_check_dict.add_to_session(kw) @@ -1274,6 +1281,9 @@ if __name__ == '__main__': print("Unknown option '%s'" % o) sys.exit(EXIT_FAILURE) + load_codespell_files() + load_codespell_words() + if sys.stdout.isatty(): colors = True
On 12/03/2025 14:40, Aaron Conole wrote: > Roi Dayan via dev <ovs-dev@openvswitch.org> writes: > >> Load dictionary_code.txt in addition to the default dictionary. >> Add a new command line argument --dictionaries to be able >> to specify codespell dictionaries. >> >> Signed-off-by: Roi Dayan <roid@nvidia.com> >> Acked-by: Salem Sol <salems@nvidia.com> >> --- > > Thanks for the update, it's close. I got a bit confused when trying to > test with this - it seems the order of arguments is dependent: > > -S -D dictionary.txt -D dictionary_rare.txt > > will not load the new dictionaries, and we have to add -S at the end. > > BUT, I think the following incremental could work to resolve that. > WDYT? Otherwise, we'll need to document that spellchecking needs to be > after the dictionaries are specified. I think the real issue is calling open_spell_check_dict() while parsing the arguments. I think of a nicer diff below which will also fix if you add -S multiple times really instead of calling open_spell_check_dict() multiple times. what do you think? I think maintaining the arguments could be easier with later moving from using getopt to python argparse. @@ -1263,11 +1264,7 @@ if __name__ == '__main__': elif o in ("-f", "--check-file"): checking_file = True elif o in ("-S", "--spellcheck"): - if not open_spell_check_dict(): - print("WARNING: The enchant library isn't available.") - print(" Please install python enchant.") - else: - spellcheck = True + spellcheck = True elif o in ("-q", "--quiet"): quiet = True else: @@ -1277,6 +1274,11 @@ if __name__ == '__main__': if sys.stdout.isatty(): colors = True + if spellcheck and not open_spell_check_dict(): + print("WARNING: The enchant library isn't available.") + print(" Please install python enchant.") + spellcheck = False + if n_patches: status = 0 > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > index dd68e307a5..6a456931a7 100755 > --- a/utilities/checkpatch.py > +++ b/utilities/checkpatch.py > @@ -40,11 +40,9 @@ missing_authors = [] > codespell_dictionaries = ['dictionary.txt', 'dictionary_code.txt'] > __codespell_dict_reset_once = True > > +codespell_files = [] > > -def open_spell_check_dict(): > - import enchant > - > - codespell_files = [] > +def load_codespell_files(): > try: > import codespell_lib > codespell_dir = os.path.dirname(codespell_lib.__file__) > @@ -55,6 +53,22 @@ def open_spell_check_dict(): > except: > pass > > + > +def load_codespell_words(): > + global spell_check_dict, codespell_files > + > + for fn in codespell_files: > + with open(fn) as f: > + for line in f.readlines(): > + words = line.strip().split('>')[1].strip(', ').split(',') > + for word in words: > + if spell_check_dict: > + spell_check_dict.add_to_session(word.strip()) > + > + > +def open_spell_check_dict(): > + import enchant > + > try: > extra_keywords = ['ovs', 'vswitch', 'vswitchd', 'ovs-vswitchd', > 'netdev', 'selinux', 'ovs-ctl', 'dpctl', 'ofctl', > @@ -125,13 +139,6 @@ def open_spell_check_dict(): > > spell_check_dict = enchant.Dict("en_US") > > - for fn in codespell_files: > - with open(fn) as f: > - for line in f.readlines(): > - words = line.strip().split('>')[1].strip(', ').split(',') > - for word in words: > - spell_check_dict.add_to_session(word.strip()) > - > for kw in extra_keywords: > spell_check_dict.add_to_session(kw) > > @@ -1274,6 +1281,9 @@ if __name__ == '__main__': > print("Unknown option '%s'" % o) > sys.exit(EXIT_FAILURE) > > + load_codespell_files() > + load_codespell_words() > + > if sys.stdout.isatty(): > colors = True > >
Roi Dayan <roid@nvidia.com> writes: > On 12/03/2025 14:40, Aaron Conole wrote: >> Roi Dayan via dev <ovs-dev@openvswitch.org> writes: >> >>> Load dictionary_code.txt in addition to the default dictionary. >>> Add a new command line argument --dictionaries to be able >>> to specify codespell dictionaries. >>> >>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>> Acked-by: Salem Sol <salems@nvidia.com> >>> --- >> >> Thanks for the update, it's close. I got a bit confused when trying to >> test with this - it seems the order of arguments is dependent: >> >> -S -D dictionary.txt -D dictionary_rare.txt >> >> will not load the new dictionaries, and we have to add -S at the end. >> >> BUT, I think the following incremental could work to resolve that. >> WDYT? Otherwise, we'll need to document that spellchecking needs to be >> after the dictionaries are specified. > > I think the real issue is calling open_spell_check_dict() while parsing the > arguments. > I think of a nicer diff below which will also fix if you add -S multiple > times really instead of calling open_spell_check_dict() multiple times. > what do you think? LGTM > I think maintaining the arguments could be easier with later moving from using > getopt to python argparse. > > @@ -1263,11 +1264,7 @@ if __name__ == '__main__': > elif o in ("-f", "--check-file"): > checking_file = True > elif o in ("-S", "--spellcheck"): > - if not open_spell_check_dict(): > - print("WARNING: The enchant library isn't available.") > - print(" Please install python enchant.") > - else: > - spellcheck = True > + spellcheck = True > elif o in ("-q", "--quiet"): > quiet = True > else: > @@ -1277,6 +1274,11 @@ if __name__ == '__main__': > if sys.stdout.isatty(): > colors = True > > + if spellcheck and not open_spell_check_dict(): > + print("WARNING: The enchant library isn't available.") > + print(" Please install python enchant.") > + spellcheck = False > + > if n_patches: > status = 0 > > > > >> >> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >> index dd68e307a5..6a456931a7 100755 >> --- a/utilities/checkpatch.py >> +++ b/utilities/checkpatch.py >> @@ -40,11 +40,9 @@ missing_authors = [] >> codespell_dictionaries = ['dictionary.txt', 'dictionary_code.txt'] >> __codespell_dict_reset_once = True >> >> +codespell_files = [] >> >> -def open_spell_check_dict(): >> - import enchant >> - >> - codespell_files = [] >> +def load_codespell_files(): >> try: >> import codespell_lib >> codespell_dir = os.path.dirname(codespell_lib.__file__) >> @@ -55,6 +53,22 @@ def open_spell_check_dict(): >> except: >> pass >> >> + >> +def load_codespell_words(): >> + global spell_check_dict, codespell_files >> + >> + for fn in codespell_files: >> + with open(fn) as f: >> + for line in f.readlines(): >> + words = line.strip().split('>')[1].strip(', ').split(',') >> + for word in words: >> + if spell_check_dict: >> + spell_check_dict.add_to_session(word.strip()) >> + >> + >> +def open_spell_check_dict(): >> + import enchant >> + >> try: >> extra_keywords = ['ovs', 'vswitch', 'vswitchd', 'ovs-vswitchd', >> 'netdev', 'selinux', 'ovs-ctl', 'dpctl', 'ofctl', >> @@ -125,13 +139,6 @@ def open_spell_check_dict(): >> >> spell_check_dict = enchant.Dict("en_US") >> >> - for fn in codespell_files: >> - with open(fn) as f: >> - for line in f.readlines(): >> - words = line.strip().split('>')[1].strip(', ').split(',') >> - for word in words: >> - spell_check_dict.add_to_session(word.strip()) >> - >> for kw in extra_keywords: >> spell_check_dict.add_to_session(kw) >> >> @@ -1274,6 +1281,9 @@ if __name__ == '__main__': >> print("Unknown option '%s'" % o) >> sys.exit(EXIT_FAILURE) >> >> + load_codespell_files() >> + load_codespell_words() >> + >> if sys.stdout.isatty(): >> colors = True >> >>
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index f8caeb811604..241346694e4b 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -37,19 +37,23 @@ spellcheck = False quiet = False spell_check_dict = None missing_authors = [] +codespell_dictionaries = ['dictionary.txt', 'dictionary_code.txt'] +__codespell_dict_reset_once = True def open_spell_check_dict(): import enchant + codespell_files = [] try: import codespell_lib codespell_dir = os.path.dirname(codespell_lib.__file__) - codespell_file = os.path.join(codespell_dir, 'data', 'dictionary.txt') - if not os.path.exists(codespell_file): - codespell_file = '' + for fn in codespell_dictionaries: + fn = os.path.join(codespell_dir, 'data', fn) + if os.path.exists(fn): + codespell_files.append(fn) except: - codespell_file = '' + pass try: extra_keywords = ['ovs', 'vswitch', 'vswitchd', 'ovs-vswitchd', @@ -121,8 +125,8 @@ def open_spell_check_dict(): spell_check_dict = enchant.Dict("en_US") - if codespell_file: - with open(codespell_file) as f: + for fn in codespell_files: + with open(fn) as f: for line in f.readlines(): words = line.strip().split('>')[1].strip(', ').split(',') for word in words: @@ -1130,6 +1134,7 @@ Check options: -a|--check-authors-file Check AUTHORS file for existence of the authors. Should be used by commiters only! -b|--skip-block-whitespace Skips the if/while/for whitespace tests +-D|--dictionaries DICTIONARY Specify codespell dictionaries. -l|--skip-leading-whitespace Skips the leading whitespace test -q|--quiet Only print error and warning information -s|--skip-signoff-lines Tolerate missing Signed-off-by line @@ -1215,10 +1220,11 @@ if __name__ == '__main__': sys.argv[1:]) n_patches = int(numeric_options[-1][1:]) if numeric_options else 0 - optlist, args = getopt.getopt(args, 'abhlstfSq', + optlist, args = getopt.getopt(args, 'abD:hlstfSq', ["check-file", "help", "check-authors-file", + "dictionaries=", "skip-block-whitespace", "skip-leading-whitespace", "skip-signoff-lines", @@ -1237,6 +1243,11 @@ if __name__ == '__main__': sys.exit(0) elif o in ("-a", "--check-authors-file"): check_authors_file = True + elif o in ("-D", "--dictionaries"): + if __codespell_dict_reset_once: + __codespell_dict_reset_once = False + codespell_dictionaries = [] + codespell_dictionaries.append(a) elif o in ("-b", "--skip-block-whitespace"): skip_block_whitespace_check = True elif o in ("-l", "--skip-leading-whitespace"):