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.

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

search: Do not separate a package from a summary with a colon #2190

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ppisar
Copy link
Contributor

@ppisar ppisar commented Apr 9, 2025

It was reported that selecting a package name from an output of "dnf5 search" command was difficult because of a colon adjacent to the pacakge name:

$ dnf5 search dontpanic
Updating and loading repositories:
Repositories loaded.
Matched fields: name (exact)
dontpanic.i686: Very simple library and executable used in testing Alien::Base

Double-clicking on "dontpanic" word selected "dontpanic.i686:", including the colon character. That made dificult to copy and paste the found packages.

While this is a problem of the terminal emulator and later the reporter found a setttings of his terminal not to do it, I think we should use a pure white-space separator to help people with not so advanced terminals.

This patch replaces the ": " separator with a single tabulation character:

$ dnf5 search dontpanic
Updating and loading repositories:
Repositories loaded.
Matched fields: name (exact)
dontpanic.i686	Very simple library and executable used in testing Alien::Base

Resolves #2166

Do we want this change?
I guess we will need to adjust dnf-behave-tests/dnf/search.feature which again matches output literally.

It was reported that selecting a package name from an output of "dnf5
search" command was difficult because of a colon adjacent to the
pacakge name:

    $ dnf5 search dontpanic
    Updating and loading repositories:
    Repositories loaded.
    Matched fields: name (exact)
    dontpanic.i686: Very simple library and executable used in testing Alien::Base

Double-clicking on "dontpanic" word selected "dontpanic.i686:",
including the colon character. That made dificult to copy and paste
the found packages.

While this is a problem of the terminal emulator and later the reporter
found a setttings of his terminal not to do it, I think we should use
a pure white-space separator to help people with not so advanced
terminals.

This patch replaces the ": " separator with a single tabulation
character:

    $ dnf5 search dontpanic
    Updating and loading repositories:
    Repositories loaded.
    Matched fields: name (exact)
    dontpanic.i686	Very simple library and executable used in testing Alien::Base

Resolves rpm-software-management#2166
@ppisar ppisar requested a review from a team as a code owner April 9, 2025 08:28
@ppisar ppisar requested review from pkratoch and removed request for a team April 9, 2025 08:28
@pkratoch
Copy link
Contributor

Yeah, I think we can change it this way. Will you prepare a patch that adjusts the tests? It affects dnf-behave-tests/dnf/search.feature and dnf-behave-tests/dnf/search-sort.feature .

@ppisar
Copy link
Contributor Author

ppisar commented Apr 10, 2025

I will prepare a patch for the tests.

@ppisar
Copy link
Contributor Author

ppisar commented Apr 10, 2025

@evan-goode
Copy link
Member

We can merge this but note there is a community PR already that changes the search output to use libsmartcols: #2180. That PR would also need accompanying changes to ci-dnf-stack.

ppisar added a commit to ppisar/ci-dnf-stack that referenced this pull request Apr 10, 2025
Implemenation detail: Bahave does not expand escape sequences in
multi-line texts associated with a step. Hence this patch uses literal
tabulation characters (U+0009) instead of "\t" sequences.

For: rpm-software-management/dnf5#2190
Resolves: rpm-software-management/dnf5#2166
@ppisar
Copy link
Contributor Author

ppisar commented Apr 10, 2025

If the smartcols-formatted output is what we want, then this pull request should not be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select / Copy / paste problem with search result.
3 participants