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

[FEATURE] Improve --list output #436

Closed
matthijskooijman opened this issue May 12, 2023 · 5 comments
Closed

[FEATURE] Improve --list output #436

matthijskooijman opened this issue May 12, 2023 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@matthijskooijman
Copy link

Describe the solution you'd like
Currently, --list outputs something like:

Using config file: config.kibot.yaml
WARNING:(W044) More than one SCH file found in `.`.
  Using ./GP20-Mainboard.kicad_sch if you want to use another use -e option. (kibot - kiplot.py:755)
Available actions:

Pre-flight:
- check_zone_fills: True
Outputs:
WARNING:(W037) Field Manufacturer of component U5 (LM22672MRE-5 LM22672MRE-5.0) contains extra spaces: `TI ` removing them. (kibot - v5_sch.py:1027)
WARNING:(W037) Field Manufacturer of component U6 (TPS2111A TPS2111) contains extra spaces: `TI ` removing them. (kibot - v5_sch.py:1027)
- 'Schematic in PDF format' (pdf_sch_print) [pdf_sch_print]
- '3D view from top' (render_3d_top) [render_3d]
- '3D view from 30 degrees' (render_3d_30deg) [render_3d]
- '3D view from bottom' (render_3d_bottom) [render_3d]

I would like to suggest to my coworkers that they can use --list to figure out what outputs kibot supports, so they can run kibot with specific target names to produce specific outputs when needed. However, I thin the --list output as shown above is not so easy to read and could be improved.

I have some specific suggestions on how to improve this:

  • Suppress warnings while running with --list. They serve no real purpose here - in fact I wonder if --list actually needs to load the kicad files at all (is it not sufficient to read the config to print this output)?
  • Change the list of outputs to more easily identify the target name by putting it in front. And maybe also remove the quotes, which I think have no clear purpose.
  • Maybe add a short message explaining what you can actually do with these?
  • Drop the "Available actions:" header, since it is not clear that "Pre-flight" and "Outputs" are intended as subheaders to that (or add some indentation to make that clear). Adding "Available" to those might also help.

Applying all suggestions, the output could be something like:

Using config file: config.kibot.yaml

Available pre-flights:
- check_zone_fills: True

Available outputs:
- pdf_sch_print: Schematic in PDF format [pdf_sch_print]
- render_3d_top: 3D view from top [render_3d]
- render_3d_30deg: 3D view from 30 degrees [render_3d]
- render_3d_bottom: 3D view from bottom [render_3d]

You can use e.g. `kibot --skip-pre preflight_name1,preflight_name2` to
skip specific preflights (or pass `all` to skip them all).

You can use e.g. `kibot output_name1 output_name1` to generate only
specific outputs by name.
@matthijskooijman matthijskooijman added the enhancement New feature or request label May 12, 2023
set-soft added a commit that referenced this issue May 17, 2023
@set-soft
Copy link
Member

Thanks for the suggestions.

  • outputs are only configured if requested
  • The Available options: line was removed and the Available word added to outputs and preflights
  • Added use examples
  • Added clarification of what the '...' (....) [...] elements are

In addition:

  • Added --only-names (currently prints only the output names)
  • Added --only-pre: prints only the preflights, can be used with --only-names to print the names of the preflights

I didn't change the 'comment/description' (name) [type] format because this is the way KiBot refers to an output every time an output object is casted to string. This includes errors.

@matthijskooijman
Copy link
Author

I didn't change the 'comment/description' (name) [type] format because this is the way KiBot refers to an output every time an output object is casted to string. This includes errors.

Would my suggestion not be easier to read on those situations as well (I only saw this format being used to print the current output in a normal run where the improved format would also work IMHO - I couldn't quickly find other places where it was used)? Or are you worried about compatibility?

In any case, the Available outputs: 'comment/description' (name) [type] header already helps to clarify (though it's still not as clear as it could be IMHO - the name is a bit hidden between the comment and type).

@matthijskooijman
Copy link
Author

I did give this a quick test - looks great otherwise!

set-soft added a commit that referenced this issue May 18, 2023
@set-soft
Copy link
Member

Would my suggestion not be easier to read on those situations as well (I only saw this format being used to print the current output in a normal run where the improved format would also work IMHO - I couldn't quickly find other places where it was used)?

Is used when reporting errors, in general: every place where one output object is printed uses it.

Or are you worried about compatibility?

Yes, and I think this is the best when the descriptions are correct.

I added an option to use the name first in the list, above mentioned commit.

@matthijskooijman
Copy link
Author

Cool, just tried it, looks good. I'll document the --output-name-first in our internal README :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants