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 hints support for checkboxes #433

Merged
merged 12 commits into from
Jan 8, 2024
Merged

Conversation

rizkyarlin
Copy link
Contributor

@rizkyarlin rizkyarlin commented Aug 1, 2023

This PR addresses #340 in this repo.

I also would like to use this feature for mac-cleanup/mac-cleanup-py#80

If this looks okay, I'll create the tests. Thank you for the awesome package!

@Cube707
Copy link
Collaborator

Cube707 commented Aug 8, 2023

Sorry I don't have much time to test this right now, but my first question is: what if a user only wants hints for some of the questions? And what if the list-length doesn't match the number of questions? Should it raise an error or just guess?

Maybe a dict would be a better choice to clearly map hints to a specific choice? Or does this lead to other problem?

Not really stuff you need to answer, just what pops into my head when looking at the changes

@rizkyarlin
Copy link
Contributor Author

I agree that dict would be better. I just followed the example from #340 . I'll update my PR later

@rizkyarlin
Copy link
Contributor Author

I updated my PR:

  1. use dict for hints
  2. add hints to List Question type
  3. add integration tests
  4. Add color for hints.
  5. Update docs/usage.md (I'm not sure about the format of the docs)

Please take a look @Cube707

@efa2d19
Copy link

efa2d19 commented Nov 25, 2023

@Cube707 any updates on when you can check it out?
This will really help us close up the issue

Copy link
Collaborator

@Cube707 Cube707 left a comment

Choose a reason for hiding this comment

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

sorry it took so long, but I now had the time to fix it all up

@Cube707 Cube707 merged commit 35e9198 into magmax:main Jan 8, 2024
13 checks passed
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.

3 participants