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 a print question type #244

Merged
merged 4 commits into from
Aug 15, 2022

Conversation

RhetTbull
Copy link
Contributor

@RhetTbull RhetTbull commented Aug 8, 2022

What is the problem that this PR addresses?
Adds print type to dictionary style prompts. Closes #207.

...

How did you solve it?
Modified prompt() to handle a print type that prints a message (with/without optional style). The name field is optional when using print. print does not return a value and does not add an entry to the returned dict from prompt(); it only outputs text. print does work with the when keyword so the message can be conditionally printed.

...

Checklist

  • I have read the Contributor's Guide.
  • I will check that all automated PR checks pass before the PR gets reviewed.

Note: several tests in tests/prompts/test_select.py failed with "too many open files". However, these same tests failed before the PR changes. I'm using a Mac running Catalina 10.15.7. All other tests (including the tests added for this PR) passed.

tests/prompts/test_select.py::test_jk_and_shortcut_conflict_avoided_by_disabling_ij_keys FAILED                                                 [ 94%]
tests/prompts/test_select.py::test_select_shortcuts FAILED                                                                                      [ 95%]
tests/prompts/test_select.py::test_select_no_arrow_keys FAILED                                                                                  [ 95%]
tests/prompts/test_select.py::test_select_no_shortcuts FAILED                                                                                   [ 96%]
tests/prompts/test_select.py::test_select_default_has_arrow_keys FAILED

mypy checks passed:

mypy questionary
Success: no issues found in 17 source files

The pycodestyle test required by contributor's guide had many failures (mostly long lines) that were already present in the source code -- not changed in this PR. I fixed the changes in the PR to ensure they introduced no new pycodestyle errors. I did add # noqa to two lines in prompt.py that were 80 and 85 lines long respectively because changing the code would have been much less clean (would require an if/then block instead of an inline which was easier to read).

Note: running black on the tests/ directory would have changed a bunch of the existing import statements so I made sure my new tests were blackified but did not run black on the existing code.

@kiancross kiancross changed the title Feature add print question type 207 Add a print question type Aug 14, 2022
@kiancross
Copy link
Collaborator

kiancross commented Aug 14, 2022

Thanks for this PR.

The pycodestyle test required by contributor's guide had many failures (mostly long lines) that were already present in the source code -- not changed in this PR.

This part of the contributor's guide has been updated (if you look at the latest version rather than stable, you will see this).

You should just be able to run make develop and then make lint to apply all formatting rules. The # noqa comments should not be necessary, as the aforementioned commands will sort out all line length issues.

Copy link
Collaborator

@kiancross kiancross left a comment

Choose a reason for hiding this comment

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

This all looks great - thanks. I've left a comment below.

questionary/prompt.py Show resolved Hide resolved
tests/test_prompt.py Outdated Show resolved Hide resolved
@kiancross kiancross enabled auto-merge (squash) August 15, 2022 22:16
@kiancross
Copy link
Collaborator

Looks good to me - thanks for the PR!

@kiancross kiancross merged commit 87a891c into tmbo:master Aug 15, 2022
@RhetTbull RhetTbull deleted the feature-add-print-question-type-207 branch August 18, 2022 12:42
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.

Add print question type to dictionary style prompt
2 participants