Skip to content

Commit

Permalink
Refactor code in repoinfo to use opts.command correctly.
Browse files Browse the repository at this point in the history
Commit de9643a (first shipped in dnf 4.2.8) changed opts.command
from a list to a simple string, but code in RepoListCommand still handled
it as if it was a list.

It turns out this code didn't really break, since the Python `in`
operator works on strings by checking the substring. So while this went
from an exact match to a substring match, in this particular case the
two will match the same cases. So `repoinfo` continued working as
expected even after the list -> string change.

Nevertheless, we should fix this code to now handle this properly as a
string and use an exact comparison instead of the substring check.

This commit also refactors some of the code, to use explicit `or` checks
rather than an `any(...)` operator with a tuple, since there was no
particular reason to use the `any(...)` form and the code didn't rely on
not short-circuiting behavior of building the tuple (none of the
conditions involved had side effects.)

Tested that the `dnf repoinfo` command works the same before and after
this change. Also tested that `dnf repoinfo --all` keeps showing all
repositories with their enabled/disabled status correctly.
  • Loading branch information
filbranden committed Apr 1, 2020
1 parent ee18313 commit 5474820
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions dnf/cli/commands/repolist.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def configure(self):
if not self.opts.quiet:
self.cli.redirect_repo_progress()
demands = self.cli.demands
if any((self.base.conf.verbose, ('repoinfo' in self.opts.command))):
if self.base.conf.verbose or self.opts.command == 'repoinfo':
demands.available_repos = True
demands.sack_activation = True

Expand Down Expand Up @@ -139,10 +139,10 @@ def run(self):
enabled = True
if arg == 'disabled':
continue
if any((include_status, verbose, 'repoinfo' in self.opts.command)):
if include_status or verbose or self.opts.command == 'repoinfo':
ui_enabled = ehibeg + _('enabled') + hiend
ui_endis_wid = exact_width(_('enabled'))
if verbose or ('repoinfo' in self.opts.command):
if verbose or self.opts.command == 'repoinfo':
ui_size = _repo_size(self.base.sack, repo)
else:
enabled = False
Expand All @@ -151,7 +151,7 @@ def run(self):
ui_enabled = dhibeg + _('disabled') + hiend
ui_endis_wid = exact_width(_('disabled'))

if not any((verbose, ('repoinfo' in self.opts.command))):
if not (verbose or self.opts.command == 'repoinfo'):
rid = ucd(repo.id)
cols.append((rid, repo.name, (ui_enabled, ui_endis_wid)))
else:
Expand Down Expand Up @@ -287,6 +287,6 @@ def run(self):
print("%s %s %s" % (fill_exact_width(rid, id_len),
fill_exact_width(rname, nm_len, nm_len),
ui_enabled))
if any((verbose, ('repoinfo' in self.opts.command))):
if verbose or self.opts.command == 'repoinfo':
msg = _('Total packages: {}')
print(msg.format(_num2ui_num(tot_num)))

0 comments on commit 5474820

Please sign in to comment.