NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.27k stars 1.48k forks source link

nix flake show: add the description if it exists #10980

Closed kjeremy closed 1 month ago

kjeremy commented 3 months ago

Motivation

Displays the description of the package if it exists. This information is already present in the json output.

I have a flake with many packages and dev shells and this helps discoverability.

Context

10977

Priorities and Process

Add :+1: to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

kjeremy commented 3 months ago

This has been really helpful for my team. What do I need to do to get it merged?

edolstra commented 3 months ago

The problem with showing descriptions in nix flake show is that they can be pretty long, which can break the tree output rendering (especially if they contain newlines).

kjeremy commented 3 months ago

@edolstra I can think of a few approaches:

  1. Strip newlines and maybe limit the length
  2. Fix the tree views to work with multiline output

I can tackle either one if I get some direction.

kjeremy commented 2 months ago

I've pushed a change to stop at the first new line

kjeremy commented 2 months ago

I'm not quite sure how to test the output. The tests in tests/functional/flakes/show.sh all depend on the json output but this needs to check stdout.

cole-h commented 2 months ago

As a bystander, I think the following implementation might make the most sense:

I'm not quite sure how to test the output. The tests in tests/functional/flakes/show.sh all depend on the json output but this needs to check stdout.

You could try using <the command> | grepQuiet "what you're looking for", I think.

kjeremy commented 2 months ago

I chose 80 and added tests.

kjeremy commented 2 months ago

@edolstra is this a reasonable solution to the tree rendering?

kjeremy commented 2 months ago

@Ericson2314 is there any thing I can do to move this along?

kjeremy commented 1 month ago

It looks like lix just merged this into their code base.

kjeremy commented 1 month ago

I think the macos test failure is spurious

tomberek commented 1 month ago

I tried this out and I still get overflow. Am I reading the logic wrong? Shouldn't the window size limitation be on the entire string rather than only the desc?

logger->cout("%s: %s '%s' - '%s'", headerPrefix, type, name, desc);
             ^-----------------^
kjeremy commented 1 month ago

I guess it could be. I was thinking it should just be on the description but maybe it should be on the whole string but we should only cut off the description if too long to match existing behavior

tomberek commented 1 month ago

Tested and works as expected. Thanks!