Skip to content

Conversation

schloerke
Copy link
Collaborator

Fixes: #1913

@schloerke schloerke requested a review from Copilot March 14, 2025 15:51
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the bookmarking documentation and integrates bookmarking-related serializer functionality into input handling. Key changes include:

  • Importing and exposing bookmark serializer functions via all.
  • Updating input handler functions to utilize the new serializer for non-bookmarkable inputs (e.g. passwords).
  • Refining comments, docstrings, and renaming internal variables to clarify bookmarking behavior.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
shiny/input_handler.py Integrates bookmark serializer into input handlers and updates documentation.
shiny/bookmark/init.py Exposes new serializer functions through updated all definitions.
shiny/bookmark/_serializers.py Adjusts serializer function parameters and formatting.
shiny/bookmark/_save_state.py Renames the on_save callback variable to _on_save for clarity.
shiny/bookmark/_bookmark.py Removes redundant TODO comments and improves docstring formatting.
Comments suppressed due to low confidence (2)

shiny/input_handler.py:160

  • [nitpick] The use of '_' as a function name for input handlers is ambiguous and may reduce code clarity; consider using a more descriptive name for these functions.
def _(value: str, name: ResolvedId, session: Session) -> str:

shiny/input_handler.py:173

  • Since the file input handler now has conditional logic for None values, please ensure that tests are added to verify the behavior when no file is provided.
    if value is None:

@schloerke schloerke enabled auto-merge (squash) March 14, 2025 15:56
@schloerke schloerke merged commit 319088e into main Mar 14, 2025
53 checks passed
@schloerke schloerke deleted the bookmarking_notes branch March 14, 2025 15:57
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.

[Docs]: Internal writeup of bookmarking implementation
1 participant