zifeo / whiz

Modern DAG/tasks runner for multi-platform monorepos with live reloading, env management, pipes, and more in a tabbed view.
https://metatype.dev/docs/reference/ecosystem?utm_source=github&utm_medium=about&utm_campaign=whiz
Mozilla Public License 2.0
19 stars 14 forks source link

feat: add graph ascii representation #103

Closed Darioazzali closed 10 months ago

Darioazzali commented 10 months ago

Added graphical ascii representation fix #100. There is the possibility too to be visualized with box-drawing character. The task resolved from the run function in main are used to build the node edge structure. Then is visualized with ratatui as independent and dependent task, the second one as an ascii graph. The keys k j l and h permits the graphic navigation through scroll, as well as the arrows. Would be grate disallow scrolling at the tasks graph boundary. If you like the solution I can do it as well. For drawing the edge and nodes I've used the termgraph crate.

zifeo commented 10 months ago

@Darioazzali Nice contribution, a little suggestion. Can you also check the errors in the tests?

Darioazzali commented 10 months ago

@zifeo sorry I've run cargo build and everything was compiling well, didn't check for the test sorry for that (in the dev branch that I've made I was using a test parsing from a big yaml and I was thinking if i should put it).

For the subcommand feature I've a couple of points to ask:

  1. I've noticed in the main branch that if your run simply whiz help or whiz help version it outputs an error: error As you can see from the first line. This is due to let args = Args::try_parse()?; https://github.com/zifeo/whiz/blob/081010c6371f069dbab880f48162ab1fa22b846a/src/main.rs#L59C24-L59C24

The error returned form the function is not kind of error, it's an error that Claps "throw" Clap DisplayHelp. I think a Args::parse() will suffice, since in case of a parsing error, clap simply quit with displaying the error or hint. Changing try_parse() with parse() solves the "issue", behaving normally if a "real" parse error is found

  1. The args.rs module adopts the Clap v3 syntax, since I'm refactoring in order to add the graph subcommand, It's ok if I switch to the new derive syntax?
    #[clap(long, value_parser)]

    to

    #[arg(long)]

    And since quoting Clap changelog

(derive) Changed the default for arguments from parse to value_parser, removing parse support (#3827, #3981)

[clap(value_parser)] and #[clap(action)] are now redundant

I would remove the [(value_parser)] too.

  1. Since I'm adding the graph subcommand should I change, for coherence, the --list-jobs from flag option to a subcommand too?
  2. The --help flags are disabled in the parser configuration
    #[clap(name="whiz", about, long_about = None, disable_version_flag = true, disable_help_flag = true)]

    Now that there is more than one subcommand (at least two with version and subgraph) should be better if this is enabled again? whiz help version whiz upgrade --help whiz upgrade -h would give the same result

Thanks for your time!

zifeo commented 10 months ago

@Darioazzali all good, very good improvement. Could you also update the readme and we are good to go?

Darioazzali commented 10 months ago

@zifeo, done!, split the subcommands from flags in the readme cli section