Den fre 28 juli 2023 kl 06:31 skrev Jun Omae <jun6...@gmail.com>: > Hi, > > On 2023/07/28 0:21, Daniel Sahlberg wrote: > > The code is: > > [[[ > > def set_log_level(option, opt, value, parser, level=None): > > if level is None: > > level = value > > parser.values.set_log_level = getattr(logging, level, None) or > int(level) > > ]]] > > > > I assume the last line should be > > [[[ > > parser.values.set_log_level = getattr(logging, "level", None) or > int(level) > > ]]] > > Unfortunately, this change is incorrect. >
I had a feeling it was too simple... The `getattr(logging, "level", None)` is to retrieve logging.level > variable. Also, the logging.level variable is unavailable. > > The callback action with set_log_level method is used for "--verbose" > option and "--set-log-level" option, however we could simply using > store_const action for "--verbose" option. > > > https://docs.python.org/3/library/optparse.html?highlight=store_const#other-actions > > [[[ > Index: build/run_tests.py > =================================================================== > --- build/run_tests.py (revision 1911320) > +++ build/run_tests.py (working copy) > @@ -1034,17 +1034,19 @@ class TestHarness: > > > def create_parser(): > - def set_log_level(option, opt, value, parser, level=None): > - if level is None: > - level = value > - parser.values.set_log_level = getattr(logging, level, None) or > int(level) > + def set_log_level(option, opt, value, parser): > + if value.isdigit(): > + value = int(value) > + else: > + value = getattr(logging, value) > + parser.values.set_log_level = value > > parser = optparse.OptionParser(usage=__doc__); > > parser.add_option('-l', '--list', action='store_true', > dest='list_tests', > help='Print test doc strings instead of running them') > - parser.add_option('-v', '--verbose', action='callback', > - callback=set_log_level, callback_args=(logging.DEBUG, > ), > + parser.add_option('-v', '--verbose', action='store_const', > + dest='set_log_level', const=logging.DEBUG, > help='Print binary command-lines') > parser.add_option('-c', '--cleanup', action='store_true', > help='Clean up after successful tests') > Index: subversion/tests/cmdline/svntest/main.py > =================================================================== > --- subversion/tests/cmdline/svntest/main.py (revision 1911320) > +++ subversion/tests/cmdline/svntest/main.py (working copy) > @@ -2188,13 +2188,12 @@ def _create_parser(usage=None): > if logger.getEffectiveLevel() == logging.NOTSET: > logger.setLevel(logging.WARN) > > - def set_log_level(option, opt, value, parser, level=None): > - if level: > - # called from --verbose > - logger.setLevel(level) > + def set_log_level(option, opt, value, parser): > + if value.isdigit(): > + level = int(value) > else: > - # called from --set-log-level > - logger.setLevel(getattr(logging, value, None) or int(value)) > + level = getattr(logging, value) > + logger.setLevel(level) > > # Set up the parser. > # If you add new options, consider adding them in > @@ -2213,10 +2212,10 @@ def _create_parser(usage=None): > help='Print test doc strings instead of running them') > parser.add_option('--milestone-filter', action='store', > dest='milestone_filter', > help='Limit --list to those with target milestone > specified') > - parser.add_option('-v', '--verbose', action='callback', > - callback=set_log_level, callback_args=(logging.DEBUG, > ), > + parser.add_option('-v', '--verbose', action='store_const', > + dest='set_log_level', const=logging.DEBUG, > help='Print binary command-lines (same as ' + > - '"--set-log-level logging.DEBUG")') > + '"--set-log-level DEBUG")') > parser.add_option('-q', '--quiet', action='store_true', > help='Print only unexpected results (not with > --verbose)') > parser.add_option('-p', '--parallel', action='store_const', > ]]] > Thanks for the explanation and link. I understand the code now and it all looks good to me (I also like that you undated main.py at the same time to have the same pattern). Can you commit this? Kind regards, Daniel Sahlberg
