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

Wrap lines of external-lib-deps hints #3616

Closed
wants to merge 1 commit into from
Closed

Wrap lines of external-lib-deps hints #3616

wants to merge 1 commit into from

Conversation

gpetiot
Copy link
Contributor

@gpetiot gpetiot commented Jul 6, 2020

Hi, I customized the pretty-printer to make it easier to copy-paste the suggested command line given as a hint (that's something I always found frustrating).

The motivation is to replace:

Hint: try: opam install capnp-rpc-lwt capnp-rpc-unix cohttp-lwt-unix current
current_ansi current_docker current_git current_github current_rpc
current_web dockerfile git-unix opam-0install prometheus prometheus-app

with

Hint: try: opam install capnp-rpc-lwt capnp-rpc-unix cohttp-lwt-unix \
current current_ansi current_docker current_git current_github current_rpc \
current_web dockerfile git-unix opam-0install prometheus prometheus-app

@gpetiot gpetiot requested a review from rgrinberg as a code owner July 6, 2020 16:59
@rgrinberg
Copy link
Member

Hi. Please note that the Pp library is maintained in this repository. In dune, we only pull from upstream periodically and you should propose any API changes upstream first.

@ghost
Copy link

ghost commented Jul 7, 2020

This is related to #3433 (which I just merged). I like the idea of still printing the command on multiple lines but using \ to quote newlines. @gpetiot if you rebase, you now have to update a single Utils.pp_command_hint function that is used consistently across the code-base.

As Rudi mentioned, we moved the development of Pp in a separate repository. It's not very obvious at the moment as Pp is still in src/stdune. This will become clearer once we "unvendor" our dependencies: #3575

BTW, I didn't know about pp_custom_break_hints but it seems like it's going to be useful to cleanup the makefile printer and a few other cases. So thanks for making me discover this function :)

@ghost
Copy link

ghost commented Jul 8, 2020

@gpetiot you can copy the changes you did in the other repository here. Sorry for the multiple hoops here, we recently started extracting a few components of general interested outside of Dune, but the workflow is not properly finalised.

@rgrinberg
Copy link
Member

I updated the pp library in dune. So it should be fine to rebase this PR.

Signed-off-by: Guillaume Petiot <guillaume@tarides.com>
Base automatically changed from master to main January 14, 2021 17:08
@gpetiot gpetiot closed this Mar 18, 2021
@gpetiot gpetiot deleted the wrap-hint-commands branch March 18, 2021 16:34
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.

2 participants