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

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

Open
10 of 11 tasks
Alizter opened this issue May 22, 2023 · 7 comments
Open
10 of 11 tasks

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

Alizter opened this issue May 22, 2023 · 7 comments
Labels
accepted accepted proposals feature-request proposal RFC's that are awaiting discussion to be accepted or rejected

Comments

@Alizter
Copy link
Collaborator

Alizter commented May 22, 2023

We should add a dune show command that will collect together many subcommands whose jobs it is to show some information about the workspace. This should make these commands easier to use and clear up the toplevel cli.

Maybe the best way to go about this is to extend dune describe and rename it to dune show. Keeping it around as a command alias seems sensible. dune describe also needs to be converted to command groups.

@Alizter Alizter added proposal RFC's that are awaiting discussion to be accepted or rejected feature-request labels May 22, 2023
@nojb
Copy link
Collaborator

nojb commented May 22, 2023

Also: dune installed-libraries should probably become dune show installed-libraries.

For dune describe there are a number of subcommands: dune describe opam-files, dune describe pp and dune describe external-lib-deps. They should all be migrated under the dune show command.

@Alizter Alizter changed the title add dune show command add dune show command (or rename dune describe?) May 22, 2023
@Alizter Alizter added this to the 3.9.0 milestone Jun 16, 2023
@Alizter
Copy link
Collaborator Author

Alizter commented Jun 17, 2023

@snowleopard What exactly do you want dune show sources to do?

@snowleopard
Copy link
Collaborator

@snowleopard What exactly do you want dune show sources to do?

Right now, if a directory contains an ignored file, a source file, and a target (generated) file, dune show targets would print the source and the target files.

I think there are possibly two issues with it.

Firstly, seeing the source file in the output may be confusing: the user asked for "targets", after all.

Secondly, there is currently no way to see just sources (still skipping ignored files and targets) or just targets (skipping ignored files and sources).

I don't necessarily advocate for adding a new command, but the current design doesn't seem great.

@snowleopard
Copy link
Collaborator

snowleopard commented Jun 18, 2023

Here is an alternative design that may be an improvement:

  • Change the command to dune show files to avoid confusion. Also works well with a possible dune show dirs we could add in future.
  • Support flags dune show files --sources-only and --targets-only to make it possible to see the corresponding subsets.

@rgrinberg rgrinberg removed this from the 3.9.0 milestone Jun 19, 2023
@Alizter
Copy link
Collaborator Author

Alizter commented Jul 5, 2023

I don't like the dune show files idea. We are declaring every buildable target to the user, this includes the source files. I would rather have an option to exclude source files such as dune show targets --no-sources. Also dune show sources seems kind of moot as this is essentially just ls at this point.

@Alizter Alizter added the accepted accepted proposals label Jul 7, 2023
@snowleopard
Copy link
Collaborator

We are declaring every buildable target to the user, this includes the source files.

Are you sure the majority of Dune users considers sources to be targets? I doubt it. Perhaps we can ask them!

That Dune copies sources into _build, and therefore happens to have build rules for them, seems somewhat accidental to me/leaking implementation details.

Also dune show sources seems kind of moot as this is essentially just ls at this point.

Build systems (Dune including) have non-trivial logic for ignoring some source files. Having a tool to expose this behaviour can be valuable for debugging; ls won't help you there.

@Alizter
Copy link
Collaborator Author

Alizter commented Jul 10, 2023

@snowleopard In #8167 I change the default behaviour of dune show targets to omit source files. I have added an --all option which will include source files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted accepted proposals feature-request proposal RFC's that are awaiting discussion to be accepted or rejected
Projects
None yet
Development

No branches or pull requests

4 participants