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

Incorrect line breaks put metavars on a new row all alone, detached from the argument #12

Closed
michelcrypt4d4mus opened this issue Sep 24, 2022 · 4 comments · Fixed by #20
Labels
bug Something isn't working

Comments

@michelcrypt4d4mus
Copy link
Contributor

michelcrypt4d4mus commented Sep 24, 2022

enjoying this library but ran into a situation where regular argparse does line breaks correctly and this does them in a way that makes it hard to understand the options. Here's the output of rich_argparse that's an issue. The example is from for the help text of the pdfalyzer, a PDF analysis tool i just released. (thess are just a snippet to show the issue but the full help text can be viewed in the pdfalyzer README here and the argparse code that creates them can be viewed here if either of those things is helpful).

Take a look at where the PCT_CONFIDENCE and STREAM_DUMP_DIR metavars end up:

FINE TUNING:
  Settings that affect aspects of the analyis and output

  --force-display-threshold            chardet.detect() scores encodings from 0-100 pct but only above this are displayed                                                                              
PCT_CONFIDENCE                                                                                                                                                                                         
  --force-decode-threshold             extremely high (AKA 'above this number') PCT_CONFIDENCE scores from chardet.detect() as to the likelihood some binary data was written with a particular        
PCT_CONFIDENCE                         encoding will cause  the pdfalyzer to do a force decode of that with that encoding. (chardet is a sophisticated libary; this is pdfalyzer's way of harnessing   
                                       that intelligence)                                                                                                                                              
FILE EXPORT:
  Export to various kinds of files
                                                                         
  -str, --extract-streams-to           extract all binary streams in the PDF to files in STREAM_DUMP_DIR then exit (requires pdf-parser.py)                                                            
STREAM_DUMP_DIR                                                                                                                                                                                        

PCT_CONFIDENCE and STREAM_DUMP_DIR are all alone in the universe with no map home.

For comparison this is how regular argparse handles it, placing the metavars on the same line as the argument they relate to.

FINE TUNING:
  Settings that affect aspects of the analyis and output

  --force-display-threshold PCT_CONFIDENCE
                        chardet.detect() scores encodings from 0-100 pct but only above this are displayed (default: 20.0)
  --force-decode-threshold PCT_CONFIDENCE
                        extremely high (AKA 'above this number') PCT_CONFIDENCE scores from chardet.detect() as to the likelihood some binary data was written with a particular encoding will cause
                        the pdfalyzer to do a force decode of that with that encoding. (chardet is a sophisticated libary; this is pdfalyzer's way of harnessing that intelligence) (default: 50.0)

FILE EXPORT:
  Export to various kinds of files

  -txt OUTPUT_DIR, --txt-output-to OUTPUT_DIR
                        write analysis to uncolored text files in OUTPUT_DIR (in addition to STDOUT)
  -svg OUTPUT_DIR, --export-svgs OUTPUT_DIR
                        export SVG images of the analysis to OUTPUT_DIR (in addition to STDOUT)
  -html OUTPUT_DIR, --export-html OUTPUT_DIR
                        export SVG images of the analysis to OUTPUT_DIR (in addition to STDOUT)
  -str STREAM_DUMP_DIR, --extract-streams-to STREAM_DUMP_DIR
                        extract all binary streams in the PDF to files in STREAM_DUMP_DIR then exit (requires pdf-parser.py)
  -pfx PREFIX, --file-prefix PREFIX
                        optional string to use as the prefix for exported files of any kind
@hamdanal
Copy link
Owner

This is by design. The rich formatter uses rich.table.Table to render the arguments and their help messages thus they cannot horizontally overlap as with the original help formatter so long arguments get folded and wrap to the next line.
That said, there is the max_help_position parameter (default=38) that you can pass to the formatter class to give more room to long arguments. Try setting it to a bigger number (I think 45 is enough in your case).

parser = argparse.ArgumentParser(
    formatter_class=lambda **k: RichHelpFormatter(max_help_position=45, **k)
)

@hamdanal hamdanal added the question Further information is requested label Sep 24, 2022
@michelcrypt4d4mus
Copy link
Contributor Author

interesting. while there's no perfect definition of "correct' i will say that as a potential installer of the package as part of my package this caused me to hold off for the time being. for those of us accustomed to dealing with arparse's "quirks" this is NBD but i could see it being a showstopper for someone who doesn't use the command line much (meaning it would cause them to fail to understand how to use the tool). but i don't have any ideas good enough to mention yet

@hamdanal
Copy link
Owner

hamdanal commented Sep 25, 2022

Yeah unfortunately this will not change. When I first started with this project, I tried to use the output of argparse.HelpFormatter and only apply coloring on top of it but soon enough it proved to be an impossible task.

I'd like to point though that max_help_position is an argparse feature and not something added by this formatter. Maybe a mention in the readme can help make this feature more discoverable. I'll look into that

Edit: I will work on changing this in the next release

@hamdanal hamdanal reopened this Sep 26, 2022
@hamdanal hamdanal added bug Something isn't working and removed question Further information is requested labels Sep 26, 2022
@michelcrypt4d4mus
Copy link
Contributor Author

i was going to suggest that instead maybe it's worth considering ditching the use of rich.Table and instead doing yr own computations line by line...
i noticed while messing around with this in dev that argparse makes two passes through the arguments (meaning it calls into your code twice for each arg) - which makes sense, it has to do that to determine the widths. but that means you could do the same sort of thing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants