feature: dune show targets and dune show aliases

[Alizter]

Add new dune show targets and dune show aliases commands that work similarly to ls displaying targets and aliases respectively in the current directory or instead any directories given as arguments. They do not show targets or aliases in subdirectories or inside directory targets. Directory targets will be printed with a trailing path separator to distinguish them however.

• fixes Diagnose missing target errors #265

and continuing

• Add a "dune targets" command to list available targets #1755

part of general effort in:

• add dune show command (or rename dune describe?) #7784

This is quite useful in cram tests to make sure dune is willing to produce the correct targets.

• changelog
• docs (more cli docs too)
• display directory targets ending with an /

[Alizter]

@emillon Since this is a relatively straightforward addition, it could perhaps be considered for 3.8.

Edit: Actually, nevermind. The scope has grown a bit so probably worth waiting to next major version.

[nojb]

Does it make sense to display aliases as well?

[Alizter]

@nojb I have something similar giving hints to aliases: #7004. However due to the way we lookup aliases we have to do it for all contexts at the same time. It shouldn't be too difficult to add an --aliases option that also shows which aliases are available.

I don't think I will be able to do it in this PR however, since it will need some more refactoring of how we handle aliases in bin. I'll open a feature request when this is merged however.

[Alizter]

@nojb Actually I went ahead and implemented an --aliases option that shows aliases in addition. Is this what you had in mind?

I'm actually unsure if we want this in the targets command or we wish to have a separate command for displaying aliases. In any case if we are undecided we can always drop that commit.

[nojb]

@nojb Actually I went ahead and implemented an --aliases option that shows aliases in addition. Is this what you had in mind?

I think so, yes, thanks! What I had in mind is a beginner; I would imagine most of the time such an user will be more interested in invoking an alias than a target file.

[snowleopard]

That's a very useful command but I feel like it shouldn't be top-level. Don't we usually put these things under dune describe?

I don't like the name "describe" and would prefer something like dune show instead, but that's a detail. I think we can end up polluting the top-level namespace if we keep adding commands like dune targets.

[nojb]

I don't like the name "describe" and would prefer something like dune show instead, but that's a detail. I think we can end up polluting the top-level namespace if we keep adding commands like dune targets.

dune show seems like a good top-level name for this kind of functionality. Why not add dune show and put the command under it? (dune show targets). Existing commands could be migrated under it as well later: dune show rules instead of dune rules, dune show env instead of dune printenv, dune show workspace instead of dune describe workspace, etc

[snowleopard]

dune show seems like a good top-level name for this kind of functionality. Why not add dune show and put the command under it? (dune show targets). Existing commands could be migrated under it as well later: dune show rules instead of dune rules, dune show env instead of dune printenv, dune show workspace instead of dune describe workspace, etc

@nojb Sounds like a great plan to me!

[snowleopard]

I'm actually unsure if we want this in the targets command or we wish to have a separate command for displaying aliases. In any case if we are undecided we can always drop that commit.

@Alizter If we go with the dune show plan, then I'd prefer aliases to be handled by dune show aliases.

[rgrinberg]

I don't like the name describe either, but I don't like to have two names for the same thing either. I'd rather teach users one bad name rather than a good name and a bad name.

[Alizter]

@rgrinberg We can deprecate the bad name (error) and point to the new good name. I've started a proposal in #7784,

[rgrinberg]

We shouldn't. The disturbance caused by this deprecation far outweighs whatever benefit can be gained from it. Going through a deprecating cycle is a ton of churn for both the developers and the users. At best, we can introduce an alias like we did for runtest.

[nojb]

We shouldn't. The disturbance caused by this deprecation far outweighs whatever benefit can be gained from it.

Even if we don't aggressively deprecate the existing commands and just alias them under new names, I think that, in terms of CLI usability, it would still be an improvement migrating to dune show as a top-level name for all these commands. We could do a "soft" deprecation by not documenting the commands under the old names anymore, so that new users would be directed to the new names only. One could even imagine going one step further (if it is possible, I haven't checked) and remove the old names from the auto-generated dune --help page. Even if that is not possible we could clearly indicate in the help page that the dune show commands are to be preferred.

[rgrinberg]

Before we deprecate (hard or soft) describe, let's come up with a simple explanation for why we're doing so. Hopefully something that a user can read in a change log and immediately see the benefit for himself.

Also, it would be a very poor experience to partition our subcommands into describe and show. The user would never remember which subcommand belongs to which group. So to follow the principle of least surprise, everything should be available under w/e group names are exposed.

[Alizter]

Regarding this PR: I've added a show subcommand and added aliases and a subcommand of that. So we have dune show aliases and dune show targets. Both have tests.

I'm drafting this PR for the time being so we can continue to discuss how best to group the commands. My thinking is that we really want to add these to dune describe and we want to rename dune describe to dune show. But let's continue that discussion in the issue.

Also dune describe isn't using command groups, so we will have to do some refactoring there first.

[snowleopard]

I just want to say that this feature was requested internally (there was particular interest for dune show aliases). I wonder if we can make progress here, somehow. Maybe add these commands both into dune describe and dune show? Alternatively, add them only to dune describe for now and create a ticket to think about a possible migration story towards dune show.

[rgrinberg]

I just want to say that this feature was requested internally (there was particular interest for dune show aliases). I wonder if we can make progress here, somehow

Externally, it's easy to make progress. We just follow existing conventions.

Internally, we no longer import the command line from upstream. Nor do we have an existing show or describe command internally, so we can name this whatever we like. However, we would need to factor this out into something that can be used by both command lines.

[rgrinberg]

test generated targets (not worth investigating at this time)

What are generated targets?

[Alizter]

test generated targets (not worth investigating at this time)

What are generated targets?

Nevermind, I was confused about something when I wrote it.

[rgrinberg]

I started doing some cleanups and realized there's a little too much copy pasting between aliases and targets for my tastes. Can you factor the code out?

[Alizter]

@rgrinberg sure, I'll have a look over the weekend.

[Alizter]

@rgrinberg I've factored the common code out, I'm not so sure about the API yet. I think there is some more cleanup we can do.

[Alizter]

I've just added a user error for when a directory is outside of the project.

[snowleopard]

Perhaps, dune show sources could be useful too: for symmetry, and for making it possible to check which files Dune actually considers sources. (Not always obvious.)

[rgrinberg]

Perhaps, dune show sources could be useful too

It could also be distinguished via some visual cue in dune show targets. We can still add dune show sources of course.

Let's save that for a subsequent PR though.

[snowleopard]

Sure, happy for this to go to a separate PR.