Clean up comments

Author: schloerke

shiny/_utils.py

@@ -281,7 +281,7 @@ async def fn_async(*args: P.args, **kwargs: P.kwargs) -> R:
     return fn_async
 
 
-# # TODO-barret-future; Q: Keep code?
+# # TODO: Barret - Future: Q: Keep code?
 # class WrapAsync(Generic[P, R]):
 #     """
 #     Make a function asynchronous.

shiny/bookmark/_bookmark.py

@@ -12,26 +12,24 @@
 from ._restore_state import RestoreContextState
 from ._save_state import ShinySaveState
 
-# TODO: bookmark button
-
-# TODO:
+# TODO: Barret - Bookmark state
 # bookmark -> save/load interface
-# * √ base class
-# * √ local
-# save/load interface -> register interface
-# * implement; Q on approach!
+# * √ global hooks
+# * √ default local functions
+# save/load interface -> register functions
+# * `shiny.bookmark.globals`
 # register interface -> Make interface for Connect
-# * implement in Connect PR
+# * TODO: implement in Connect PR
 # bookmark -> save state
 # save state -> {inputs, values, exclude}
 # {inputs} -> custom serializer
-# √ Hook to `Inputs.set_serializer(id, fn)`
-# √ `Inputs._serialize()` to create a dict
+# * √ Hook to `Inputs.set_serializer(id, fn)`
+# * √ `Inputs._serialize()` to create a dict
 # {values} -> dict (where as in R is an environment)
-# √ values is a dict!
+# * √ values is a dict!
 # {exclude} -> Requires `session.setBookmarkExclude(names)`, `session.getBookmarkExclude()`
-# √ `session.bookmark_exclude: list[str]` value!
-# √ `session._get_bookmark_exclude()` & `session._bookmark_exclude_fn`
+# * √ `session.bookmark_exclude: list[str]` value!
+# * √ `session._get_bookmark_exclude()` & `session._bookmark_exclude_fn`
 # Using a `.bookmark_exclude = []` and `._get_bookmark_exclude()` helper that accesses a `._bookmark_exclude_fns` list of functions which return scoped bookmark excluded values
 # Enable bookmarking hooks:
 # * √ `session.bookmark_store`: `url`, `server`, `disable`
@@ -485,8 +483,6 @@ async def root_state_on_save(state: ShinySaveState) -> None:
             from ..ui._notification import notification_show
 
             notification_show(msg, duration=None, type="error")
-            # TODO: Barret - Remove this!
-            raise RuntimeError("Error bookmarking state") from e
 
 
 class BookmarkProxy(Bookmark):

shiny/bookmark/_globals.py

@@ -6,11 +6,15 @@
 from ..types import MISSING, MISSING_TYPE
 
 # TODO: Barret: Q: Can we merge how bookmark dirs are saved / loaded?... it's the same directory! However, the save will return a possibly new path. Restoring will return an existing path.
-BookmarkStateSaveDir = Callable[[str], Awaitable[Path]]
-BookmarkStateLoadDir = Callable[[str], Awaitable[Path]]
-bookmark_state_save_dir: BookmarkStateSaveDir | MISSING_TYPE = MISSING
-bookmark_state_load_dir: BookmarkStateLoadDir | MISSING_TYPE = MISSING
+BookmarkSaveDir = Callable[[str], Awaitable[Path]]
+BookmarkLoadDir = Callable[[str], Awaitable[Path]]
+
+bookmark_save_dir: BookmarkSaveDir | MISSING_TYPE = MISSING
+bookmark_load_dir: BookmarkLoadDir | MISSING_TYPE = MISSING
 
 BookmarkStore = Literal["url", "server", "disable"]
-# TODO: barret - Q: Should we have a `enable_bookmarking(store: BookmarkStore)` function?
+# TODO: Barret - Q: Should we have a `enable_bookmarking(store: BookmarkStore)` function?
 bookmark_store: BookmarkStore = "disable"
+
+
+# TODO: Barret; Q: I feel like there could be a `@shiny.globals.on_session_start` decorator that would allow us to set these values.

shiny/bookmark/_restore_state.py

@@ -12,7 +12,7 @@
 
 from . import _globals as bookmark_globals
 from ._bookmark_state import local_load_dir
-from ._globals import BookmarkStateLoadDir
+from ._globals import BookmarkLoadDir
 from ._utils import from_json_str, is_hosted
 
 
@@ -186,13 +186,13 @@ async def _load_state_qs(self, query_string: str) -> None:
 
         id = id[0]
 
-        load_bookmark_fn: BookmarkStateLoadDir | None = None
-        if not isinstance(bookmark_globals.bookmark_state_load_dir, MISSING_TYPE):
-            load_bookmark_fn = bookmark_globals.bookmark_state_load_dir
+        load_bookmark_fn: BookmarkLoadDir | None = None
+        if not isinstance(bookmark_globals.bookmark_load_dir, MISSING_TYPE):
+            load_bookmark_fn = bookmark_globals.bookmark_load_dir
 
         if load_bookmark_fn is None:
             if is_hosted():
-                # TODO: Barret
+                # TODO: Barret; Implement Connect's `bookmark_load_dir` function
                 raise NotImplementedError(
                     "The hosting environment does not support server-side bookmarking."
                 )

shiny/bookmark/_save_state.py

@@ -10,10 +10,10 @@
 from ..types import MISSING_TYPE
 from . import _globals as bookmark_globals
 from ._bookmark_state import local_save_dir
-from ._globals import BookmarkStateSaveDir
+from ._globals import BookmarkSaveDir
 from ._utils import is_hosted, to_json_str
 
-# TODO: barret - Set / Load SaveState for Connect. Ex: Connect https://github.com/posit-dev/connect/blob/8de330aec6a61cf21e160b5081d08a1d3d7e8129/R/connect.R#L915
+# TODO: Barret - Set / Load SaveState for Connect. Ex: Connect https://github.com/posit-dev/connect/blob/8de330aec6a61cf21e160b5081d08a1d3d7e8129/R/connect.R#L915
 # Might need to have independent save/load functions to register to avoid a class constructor
 
 
@@ -76,13 +76,13 @@ async def _save_state(self) -> str:
         # to `self.dir`.
 
         # This will be defined by the hosting environment if it supports bookmarking.
-        save_bookmark_fn: BookmarkStateSaveDir | None = None
-        if not isinstance(bookmark_globals.bookmark_state_save_dir, MISSING_TYPE):
-            save_bookmark_fn = bookmark_globals.bookmark_state_save_dir
+        save_bookmark_fn: BookmarkSaveDir | None = None
+        if not isinstance(bookmark_globals.bookmark_save_dir, MISSING_TYPE):
+            save_bookmark_fn = bookmark_globals.bookmark_save_dir
 
         if save_bookmark_fn is None:
             if is_hosted():
-                # TODO: Barret
+                # TODO: Barret; Implement `bookmark_save_dir` for Connect
                 raise NotImplementedError(
                     "The hosting environment does not support server-side bookmarking."
                 )

shiny/bookmark/_serializers.py

@@ -27,15 +27,15 @@ async def serializer_default(value: T, state_dir: Path | None) -> T:
     return value
 
 
-# TODO-barret; Integrate
+# TODO: Barret - Integrate
 def serializer_file_input(
     value: Any,
     state_dir: Path | None,
 ) -> Any | Unserializable:
     if state_dir is None:
         return Unserializable()
 
-    # TODO: barret; Double check this logic!
+    # TODO: Barret - Double check this logic!
 
     # `value` is a data frame. When persisting files, we need to copy the file to
     # the persistent dir and then strip the original path before saving.

shiny/bookmark/_utils.py

@@ -9,7 +9,7 @@
 def is_hosted() -> bool:
     # Can't look at SHINY_PORT, as we already set it in shiny/_main.py's `run_app()`
 
-    # TODO: Support shinyapps.io or use `SHINY_PORT` how R-shiny did
+    # TODO: Barret: Q: How to support shinyapps.io? Or use `SHINY_PORT` how R-shiny did
 
     # Instead, looking for the presence of the environment variable that Connect sets
     # (*_Connect) or Shiny Server sets (SHINY_APP)

shiny/express/_stub_session.py

@@ -47,9 +47,6 @@ def __init__(self, ns: ResolvedId = Root):
 
         self.bookmark = BookmarkExpressStub(self)
 
-        self.exclude = []
-        self.store = "disable"  # TODO: Is this correct?
-
     def is_stub_session(self) -> Literal[True]:
         return True
 

shiny/input_handler.py

@@ -151,12 +151,14 @@ def _(value: str, name: ResolvedId, session: Session) -> str:
 
 
 # TODO: implement when we have bookmarking
+# TODO: Barret: Input handler for passwords
 @input_handlers.add("shiny.password")
 def _(value: str, name: ResolvedId, session: Session) -> str:
     return value
 
 
 # TODO: implement when we have bookmarking
+# TODO: Barret: Input handler for file inputs
 @input_handlers.add("shiny.file")
 def _(value: Any, name: ResolvedId, session: Session) -> Any:
     return value

shiny/session/_session.py

@@ -1457,7 +1457,7 @@ async def _serialize(
         with reactive.isolate():
 
             for key, value in self._map.items():
-                # TODO: Barret - Q: Should this be anything that starts with a "."?
+                # TODO: Barret - Q: Should this be ignoring any Input key that starts with a "."?
                 if key.startswith(".clientdata_"):
                     continue
                 if key == BOOKMARK_ID or key.endswith(