Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better help organization for --querytags and --queryinfo #1627

Merged
merged 1 commit into from
Jun 25, 2020
Merged

Better help organization for --querytags and --queryinfo #1627

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions dnf/cli/commands/repoquery.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,12 @@
# matches %[-][dd]{attr}
QFORMAT_MATCH = re.compile(r'%(-?\d*?){([:.\w]+?)}')

QUERY_TAGS = """
QUERY_TAGS = """\
name, arch, epoch, version, release, reponame (repoid), evr,
debug_name, source_name, source_debug_name,
installtime, buildtime, size, downloadsize, installsize,
provides, requires, obsoletes, conflicts, sourcerpm,
description, summary, license, url, reason
"""
description, summary, license, url, reason"""

OPTS_MAPPING = {
'conflicts': 'conflicts',
Expand Down Expand Up @@ -165,9 +164,6 @@ def set_argparser(parser):
'used with --whatrequires, and --requires --resolve, query packages recursively.'))
parser.add_argument('--deplist', action='store_true', help=_(
"show a list of all dependencies and what packages provide them"))
parser.add_argument('--querytags', action='store_true',
help=_('show available tags to use with '
'--queryformat'))
parser.add_argument('--resolve', action='store_true',
help=_('resolve capabilities to originating package(s)'))
parser.add_argument("--tree", action="store_true",
Expand Down Expand Up @@ -195,7 +191,12 @@ def set_argparser(parser):
help=_('show changelogs of the package'))
outform.add_argument('--qf', "--queryformat", dest='queryformat',
default=QFORMAT_DEFAULT,
help=_('format for displaying found packages'))
help=_('display format for listing packages: '
'"%%{name} %%{version} ...", '
'use --querytags to view full tag list'))
abitrolly marked this conversation as resolved.
Show resolved Hide resolved
parser.add_argument('--querytags', action='store_true',
help=_('show available tags to use with '
'--queryformat'))
outform.add_argument("--nevra", dest='queryformat', const=QFORMAT_DEFAULT,
action='store_const',
help=_('use name-epoch:version-release.architecture format for '
Expand Down Expand Up @@ -433,7 +434,6 @@ def _add_add_remote_packages(self):

def run(self):
if self.opts.querytags:
print(_('Available query-tags: use --queryformat ".. %{tag} .."'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine personally with removing this, but I'd like a second opinion just to make sure someone doesn't find it actually userful, @dmach @j-mracek @m-blaha, do you agree with removing this line?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if you wanna do this, could you also remove at least the leading line-break (and ideally the trailing one too) from the QUERY_TAGS variable? Now it prints an empty line before and after the list of tags which are superfluous.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed spaces. Although I would also split reponame (repoid) into separate items, or removed reponame completely. Then it will be relatively easy to parse the output with something like bash.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears reponame doesn't stand for the name, it's the id, same as repoid. An obvious error? I'm not against just removing it, but I'm not sure we can do that. I'm sure @j-mracek will know more though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry, but I do not understanding your question. Reponame as an attribute of package class. --qf is capable to return any attribute of Package class.

  .. attribute:: reponame

    Id of repository the package was installed from (string).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry, but I do not understanding your question. Reponame as an attribute of package class. --qf is capable to return any attribute of Package class.

The question is whether we should remove reponame from the list, because it contains the ID, not the name. And since there already is repoid, it is redundant in addition to wrong.

  .. attribute:: reponame

    Id of repository the package was installed from (string).

I'd assume this is some sort of legacy API attribute, since it's called "name" and contains the ID. FTR the excerpt is from the API documentation: https://github.com/rpm-software-management/dnf/blob/master/doc/api_package.rst. Interestingly repoid isn't listed there and also no way to get the actual repo name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any benefit in removal of print(_('Available query-tags: use --queryformat ".. %{tag} .."'))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@j-mracek it is to make the output suitable for scripting. For example https://github.com/rpm-software-management/libdnf/issues/720#issuecomment-631625069

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I'm not entirely sure what would you parse the list for (ultimately you have to know the semantics of each field so you need to have them listed somewhere in your code anyway), I find it reasonable to remove it...

print(QUERY_TAGS)
return

Expand Down