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

search: Don't omit version info when package name is matched #3055

Closed
wants to merge 1 commit into from
Closed

search: Don't omit version info when package name is matched #3055

wants to merge 1 commit into from
+1 −1

Conversation

lilyball
Copy link
Member

@lilyball lilyball commented Aug 23, 2019 •
edited
Loading

The name variable already includes highlighting in addition to the version info, so with the old code it would print the version info only if nameMatch is empty.

Fixes #2778.

@lilyball
Copy link
Member Author

CC @dtzWill

@lilyball
Copy link
Member Author

I tested locally and confirmed that it still highlights the match in the name.

Screen Shot 2019-08-23 at 12 28 44 PM

@unode
Copy link
Member

unode commented Mar 20, 2020

Anything holding this merge back? Just bumped into this issue too.

@blinry
Copy link

blinry commented May 24, 2020

Me too! I think that merging this could avoid a lot of confusion for beginners.

@smancill smancill mentioned this pull request Oct 29, 2020
Open
@stale
Copy link

stale bot commented Feb 12, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Feb 12, 2021
@lilyball
Copy link
Member Author

I still want this. Didn’t realize there was a merge conflict though. I’ll have to look at that when I get a chance.

@stale stale bot removed the stale label Feb 12, 2021
@stale
Copy link

stale bot commented Aug 13, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Aug 13, 2021
@lilyball
Copy link
Member Author

lilyball commented Sep 7, 2021

I'm not really sure what the state of nix search is on master right now. I especially don't know how to get it to reproduce the Nix 2.3 behavior of searching my channel (experimentally I can do something like nix search -f '<nixpkgs>' foo but this seems to just look up nixpkgs.foo instead of searching). Also, testing this right now, it's printing the version but not the name!

Looking at the code right now, it looks like the version info is indeed fixed, in that it now adds ($version) if the version is non-empty. But the code that's supposed to print the attribute is broken (at least, in the nix search -f '<nixpkgs>' foo invocation, probably because it's not actually searching but just looking up a specific attribute). And it's not printing the name (it calculates the name highlight and then throws it away).

nix/src/nix/search.cc

Lines 134 to 142 in 6678e98

auto name2 = hilite(name.name, nameMatch, "\e[0;2m");
if (results > 1) logger->cout("");
logger->cout(
"* %s%s",
wrap("\e[0;1m", hilite(attrPath2, attrPathMatch, "\e[0;1m")),
name.version != "" ? " (" + name.version + ")" : "");
if (description != "")
logger->cout(
" %s", hilite(description, descriptionMatch, ANSI_NORMAL));

It does appear that "don't omit version info" is indeed fixed in master (but not in 2.3-maintenance), but it's been traded for other breakage. Either way I'm going to close out this issue as the other breakage is orthogonal to the version.

@lilyball lilyball closed this Sep 7, 2021
@lilyball lilyball deleted the nix_search_match_package_name branch September 7, 2021 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

nix search drops version when search term matches package name
3 participants
@lilyball @unode @blinry