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 links to the query api page in view and find --help #761

Merged
merged 7 commits into from
May 20, 2022

Conversation

cbkerr
Copy link
Member

@cbkerr cbkerr commented May 19, 2022

Description

Adds plaintext links to the output of signac view --help and signac find --help

Motivation and Context

I and others often can't find the query api page from the docs about finding and viewing.

Tested by building locally.

Checklist:

@cbkerr cbkerr requested a review from Charlottez112 May 19, 2022 16:38
@cbkerr cbkerr requested review from a team as code owners May 19, 2022 16:38
@cbkerr cbkerr requested review from mikemhenry and removed request for a team May 19, 2022 16:38
@cbkerr cbkerr changed the title Add links to the query api for view and find Add links to the query api page in view and find --help May 19, 2022
@@ -1469,8 +1469,10 @@ def main():
parser_find = subparsers.add_parser(
"find",
description="""All filter arguments may be provided either directly in JSON
Copy link
Member

@bdice bdice May 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it could be helpful to use some intentional line breaks in this message. I looked at man pages for a few executables and commands like apt search --help, docker run --help and including line breaks is very common for longer descriptions like this. I'd recommend the following:

Filter arguments may be provided directly as JSON or in a simplified form using the query API. For, example the following two bash commands are equivalent:

$ signac find key 42
$ signac find '{"a": 42}'

For more information, see: https://docs.signac.io/en/latest/query.html#simplified-filter

Note that we have an anchor in the source that can be used to shorten this URL: https://github.com/glotzerlab/signac-docs/blame/ddd4a06b23f82b722303d31b3d1eb7338a2fcd97/docs/source/query.rst#L251

I'd do something similar for the other case, too. This is helpful, thanks!

Copy link
Member Author

@cbkerr cbkerr May 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. The included argparse.RawDescriptionHelpFormatter and argparse.RawTextHelpFormatter don't allow wrapping of some text while preserving blank lines. There is a long running feature request for a better formatter for argparse.

I propose adding a new dependency argparse-formatter and its FlexiFormatter, which has been proposed for inclusion in Python, until it gets merged.

See the source of the formatter for docs.

@cbkerr cbkerr requested a review from bdice May 20, 2022 03:56
@cbkerr cbkerr requested a review from a team as a code owner May 20, 2022 04:01
doc/requirements.txt Outdated Show resolved Hide resolved
signac/__main__.py Outdated Show resolved Hide resolved
@bdice
Copy link
Member

bdice commented May 20, 2022

@cbkerr For some context: you may have observed from making this PR that argparse has some undesirable and awkward nuances in its design, and I think many Python developers feel the same way about it. However, it's the only real option we have in the standard library (forgoing the deprecated optparse). The most popular package beyond the standard library for CLI design is click. Previous efforts to rewrite signac's CLI using click stalled because of some nuances in the signac CLI that click didn't support. See conversations on #309. We might be able to adopt click in signac 2.0, if someone is passionate about doing that, but it has not been a high priority task in the past.

@cbkerr cbkerr requested a review from bdice May 20, 2022 15:54
@bdice bdice enabled auto-merge (squash) May 20, 2022 17:16
@bdice bdice merged commit 793f160 into master May 20, 2022
@bdice bdice deleted the fix/cli-link-query-api branch May 20, 2022 17:33
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