Skip to content

feat(picker): support negative default_index - #166

Open
miskatul-anwar wants to merge 1 commit into
aisk:masterfrom
miskatul-anwar:feat/negative-index-select-all-and-disabled-fix
Open

feat(picker): support negative default_index#166
miskatul-anwar wants to merge 1 commit into
aisk:masterfrom
miskatul-anwar:feat/negative-index-select-all-and-disabled-fix

Conversation

@miskatul-anwar

@miskatul-anwar miskatul-anwar commented Aug 11, 2026

Copy link
Copy Markdown

Summary

This PR adds support for negative default_index values (e.g. default_index=-1 to select the last option by default), following standard Python negative indexing conventions.

  • Bounds checking ensures -len(options) <= default_index < len(options).
  • Negative indices are converted to their corresponding positive index upon initialization.
  • Added comprehensive unit tests in tests/test_pick.py.

Testing

  • Ran pytest test suite with all tests passing.
  • Ran mypy type checks with 0 errors.

@aisk

aisk commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Why the try/except around cadule? It's already declared in pyproject.toml, so if it's missing the install is already broken, and this just turns a clear ImportError into a confusing TypeError later on.

Also this PR is doing too many unrelated things at once. Please split it up, one PR per feature.

@miskatul-anwar
miskatul-anwar force-pushed the feat/negative-index-select-all-and-disabled-fix branch from 4e6190d to 86676b7 Compare August 23, 2026 18:51
@miskatul-anwar miskatul-anwar changed the title feat(picker): support negative default_index, select_all/deselect_all, and prevent marking disabled options feat(picker): support negative default_index Aug 23, 2026
@miskatul-anwar

Copy link
Copy Markdown
Author

Thanks for the review @aisk!

I've addressed both points:

  1. Removed the unnecessary try...except block around cadule.
  2. Scoped this PR solely to the negative default_index support feature.

I will open separate PRs for the disabled option safety fix in mark_index() and the select_all() / deselect_all() helper methods.

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