From 3e30ea8ca7a36548a0addc00ad467992dddc4095 Mon Sep 17 00:00:00 2001
From: Winston Chang <winston@posit.co>
Date: Tue, 12 Mar 2024 17:27:46 -0500
Subject: [PATCH 1/7] Allow Shiny Express syntax in Quarto Dashboards

---
 shiny/express/__init__.py    | 10 ++++++++++
 shiny/express/_is_express.py | 20 +++++++++++++++++++-
 shiny/express/_run.py        | 11 +++++++++--
 shiny/quarto.py              |  3 +++
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/shiny/express/__init__.py b/shiny/express/__init__.py
index aa9cd1fa0..633044de0 100644
--- a/shiny/express/__init__.py
+++ b/shiny/express/__init__.py
@@ -37,6 +37,16 @@
 session: _Session
 
 
+allow_express_in_core = False
+"""
+Control whether Shiny Express imports are allowed in Shiny Core apps.
+
+Normally Shiny Express imports are not allowed in Shiny Core apps because the mixing of
+the two could lead to confusion. However, there are special cases where we do allow
+this, like when using Shiny Express syntax in a Quarto Dashboard.
+"""
+
+
 # Note that users should use `from shiny.express import input` instead of `from shiny
 # import express` and acces via `express.input`. The former provides a static value for
 # `input`, but the latter is dynamic -- every time `express.input` is accessed, it
diff --git a/shiny/express/_is_express.py b/shiny/express/_is_express.py
index f2f1b643d..589192b6a 100644
--- a/shiny/express/_is_express.py
+++ b/shiny/express/_is_express.py
@@ -51,13 +51,17 @@ def is_express_app(app: str, app_dir: str | None) -> bool:
     except Exception:
         return False
 
-    return detector.found_shiny_express_import
+    return detector.is_express_app()
 
 
 class DetectShinyExpressVisitor(ast.NodeVisitor):
     def __init__(self):
         super().__init__()
         self.found_shiny_express_import = False
+        self.found_shiny_express_in_core = False
+
+    def is_express_app(self) -> bool:
+        return self.found_shiny_express_import and not self.found_shiny_express_in_core
 
     def visit_Import(self, node: ast.Import) -> None:
         if any(alias.name == "shiny.express" for alias in node.names):
@@ -71,6 +75,20 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
         ):
             self.found_shiny_express_import = True
 
+    def visit_Assign(self, node: ast.Assign) -> None:
+        for target in node.targets:
+            # Look for the statement `shiny.express.allow_express_in_core = True` at the
+            # top level.
+            if (
+                isinstance(target, ast.Attribute)
+                and isinstance(target.value, ast.Attribute)
+                and target.value.attr == "express"
+                and isinstance(target.value.value, ast.Name)
+                and target.value.value.id == "shiny"
+                and target.attr == "allow_express_in_core"
+            ):
+                self.found_shiny_express_in_core = True
+
     # Visit top-level nodes.
     def visit_Module(self, node: ast.Module) -> None:
         super().generic_visit(node)
diff --git a/shiny/express/_run.py b/shiny/express/_run.py
index 53f112d25..f55eef3ca 100644
--- a/shiny/express/_run.py
+++ b/shiny/express/_run.py
@@ -144,8 +144,15 @@ def set_result(x: object):
         get_top_level_recall_context_manager().__exit__(None, None, None)
 
         # If we're running as an Express app but there's also a top-level item named app
-        # which is a shiny.App object, the user probably made a mistake.
-        if "app" in var_context and isinstance(var_context["app"], App):
+        # which is a shiny.App object, the user probably made a mistake. The exception
+        # is if allow_express_in_core is True, which happens with Quarto Dashboards.
+        from . import allow_express_in_core
+
+        if (
+            not allow_express_in_core
+            and "app" in var_context
+            and isinstance(var_context["app"], App)
+        ):
             raise RuntimeError(
                 "This looks like a Shiny Express app because it imports shiny.express, "
                 "but it also looks like a Shiny Core app because it has a variable named "
diff --git a/shiny/quarto.py b/shiny/quarto.py
index 5f0d5efc2..a00270eaa 100644
--- a/shiny/quarto.py
+++ b/shiny/quarto.py
@@ -73,6 +73,9 @@ def convert_code_cells_to_app_py(json_file: str | Path, app_file: str | Path) ->
 
 from pathlib import Path
 from shiny import App, Inputs, Outputs, Session, ui
+import shiny.express
+
+shiny.express.allow_express_in_core = True
 
 {"".join(global_code_cell_texts)}
 

From d16ca4763cda3ecb8d268360f606f43fa2610291 Mon Sep 17 00:00:00 2001
From: Winston Chang <winston@posit.co>
Date: Wed, 13 Mar 2024 16:57:07 -0500
Subject: [PATCH 2/7] Use magic comment

---
 shiny/express/__init__.py    | 10 ----------
 shiny/express/_is_express.py | 29 ++++++++++-------------------
 shiny/express/_run.py        | 11 ++---------
 shiny/quarto.py              |  4 +---
 4 files changed, 13 insertions(+), 41 deletions(-)

diff --git a/shiny/express/__init__.py b/shiny/express/__init__.py
index 633044de0..aa9cd1fa0 100644
--- a/shiny/express/__init__.py
+++ b/shiny/express/__init__.py
@@ -37,16 +37,6 @@
 session: _Session
 
 
-allow_express_in_core = False
-"""
-Control whether Shiny Express imports are allowed in Shiny Core apps.
-
-Normally Shiny Express imports are not allowed in Shiny Core apps because the mixing of
-the two could lead to confusion. However, there are special cases where we do allow
-this, like when using Shiny Express syntax in a Quarto Dashboard.
-"""
-
-
 # Note that users should use `from shiny.express import input` instead of `from shiny
 # import express` and acces via `express.input`. The former provides a static value for
 # `input`, but the latter is dynamic -- every time `express.input` is accessed, it
diff --git a/shiny/express/_is_express.py b/shiny/express/_is_express.py
index 589192b6a..9a0c03c8c 100644
--- a/shiny/express/_is_express.py
+++ b/shiny/express/_is_express.py
@@ -44,6 +44,15 @@ def is_express_app(app: str, app_dir: str | None) -> bool:
         # Read the file, parse it, and look for any imports of shiny.express.
         with open(app_path) as f:
             content = f.read()
+
+        # Check for magic comment in the first 1000 characters
+        for line in content[:1000].splitlines():
+            line = line.rstrip()
+            if line == "# shiny_mode: express":
+                return True
+            elif line == "# shiny_mode: core":
+                return False
+
         tree = ast.parse(content, app_path)
         detector = DetectShinyExpressVisitor()
         detector.visit(tree)
@@ -51,17 +60,13 @@ def is_express_app(app: str, app_dir: str | None) -> bool:
     except Exception:
         return False
 
-    return detector.is_express_app()
+    return detector.found_shiny_express_import
 
 
 class DetectShinyExpressVisitor(ast.NodeVisitor):
     def __init__(self):
         super().__init__()
         self.found_shiny_express_import = False
-        self.found_shiny_express_in_core = False
-
-    def is_express_app(self) -> bool:
-        return self.found_shiny_express_import and not self.found_shiny_express_in_core
 
     def visit_Import(self, node: ast.Import) -> None:
         if any(alias.name == "shiny.express" for alias in node.names):
@@ -75,20 +80,6 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
         ):
             self.found_shiny_express_import = True
 
-    def visit_Assign(self, node: ast.Assign) -> None:
-        for target in node.targets:
-            # Look for the statement `shiny.express.allow_express_in_core = True` at the
-            # top level.
-            if (
-                isinstance(target, ast.Attribute)
-                and isinstance(target.value, ast.Attribute)
-                and target.value.attr == "express"
-                and isinstance(target.value.value, ast.Name)
-                and target.value.value.id == "shiny"
-                and target.attr == "allow_express_in_core"
-            ):
-                self.found_shiny_express_in_core = True
-
     # Visit top-level nodes.
     def visit_Module(self, node: ast.Module) -> None:
         super().generic_visit(node)
diff --git a/shiny/express/_run.py b/shiny/express/_run.py
index f55eef3ca..53f112d25 100644
--- a/shiny/express/_run.py
+++ b/shiny/express/_run.py
@@ -144,15 +144,8 @@ def set_result(x: object):
         get_top_level_recall_context_manager().__exit__(None, None, None)
 
         # If we're running as an Express app but there's also a top-level item named app
-        # which is a shiny.App object, the user probably made a mistake. The exception
-        # is if allow_express_in_core is True, which happens with Quarto Dashboards.
-        from . import allow_express_in_core
-
-        if (
-            not allow_express_in_core
-            and "app" in var_context
-            and isinstance(var_context["app"], App)
-        ):
+        # which is a shiny.App object, the user probably made a mistake.
+        if "app" in var_context and isinstance(var_context["app"], App):
             raise RuntimeError(
                 "This looks like a Shiny Express app because it imports shiny.express, "
                 "but it also looks like a Shiny Core app because it has a variable named "
diff --git a/shiny/quarto.py b/shiny/quarto.py
index a00270eaa..380c354cf 100644
--- a/shiny/quarto.py
+++ b/shiny/quarto.py
@@ -68,14 +68,12 @@ def convert_code_cells_to_app_py(json_file: str | Path, app_file: str | Path) ->
             )
 
     app_content = f"""# This file generated by Quarto; do not edit by hand.
+# shiny_mode: core
 
 from __future__ import annotations
 
 from pathlib import Path
 from shiny import App, Inputs, Outputs, Session, ui
-import shiny.express
-
-shiny.express.allow_express_in_core = True
 
 {"".join(global_code_cell_texts)}
 

From 0f2eb0489cee5280f9247dcf3d5bab6366eff905 Mon Sep 17 00:00:00 2001
From: Winston Chang <winston@posit.co>
Date: Wed, 13 Mar 2024 17:18:44 -0500
Subject: [PATCH 3/7] Raise express/core error in more specific case

---
 shiny/express/_is_express.py | 34 ++++++++++++++++++++++++++++------
 shiny/express/_run.py        | 11 +++++++++--
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/shiny/express/_is_express.py b/shiny/express/_is_express.py
index 9a0c03c8c..04ac4b378 100644
--- a/shiny/express/_is_express.py
+++ b/shiny/express/_is_express.py
@@ -5,7 +5,9 @@
 from __future__ import annotations
 
 import ast
+import re
 from pathlib import Path
+from typing import Literal
 
 from .._docstring import no_example
 
@@ -46,12 +48,11 @@ def is_express_app(app: str, app_dir: str | None) -> bool:
             content = f.read()
 
         # Check for magic comment in the first 1000 characters
-        for line in content[:1000].splitlines():
-            line = line.rstrip()
-            if line == "# shiny_mode: express":
-                return True
-            elif line == "# shiny_mode: core":
-                return False
+        forced_mode = find_magic_comment_mode(content[:1000])
+        if forced_mode == "express":
+            return True
+        elif forced_mode == "core":
+            return False
 
         tree = ast.parse(content, app_path)
         detector = DetectShinyExpressVisitor()
@@ -87,3 +88,24 @@ def visit_Module(self, node: ast.Module) -> None:
     # Don't recurse into any nodes, so the we'll only ever look at top-level nodes.
     def generic_visit(self, node: ast.AST) -> None:
         pass
+
+
+def find_magic_comment_mode(content: str) -> Literal["core", "express"] | None:
+    """
+    Look for a magic comment of the form "# shiny_mode: express" or "# shiny_mode:
+    core".
+
+    Returns
+    -------
+    :
+        `True` if Shiny Express comment is found, `False` if Shiny Core comment is
+        found, and `None` if no magic comment is found.
+    """
+
+    for line in content[:1000].splitlines():
+        if re.search(r"^#\s*shiny_mode:\s*express\s*$", line):
+            return "express"
+        if re.search(r"^#\s*shiny_mode:\s*core\s*$", line):
+            return "core"
+
+    return None
diff --git a/shiny/express/_run.py b/shiny/express/_run.py
index 53f112d25..d272b11b7 100644
--- a/shiny/express/_run.py
+++ b/shiny/express/_run.py
@@ -13,6 +13,7 @@
 from .._utils import import_module_from_path
 from ..session import Inputs, Outputs, Session, get_current_session, session_context
 from ..types import MISSING, MISSING_TYPE
+from ._is_express import find_magic_comment_mode
 from ._mock_session import ExpressMockSession
 from ._recall_context import RecallContextManager
 from .expressify_decorator._func_displayhook import _expressify_decorator_function_def
@@ -144,8 +145,14 @@ def set_result(x: object):
         get_top_level_recall_context_manager().__exit__(None, None, None)
 
         # If we're running as an Express app but there's also a top-level item named app
-        # which is a shiny.App object, the user probably made a mistake.
-        if "app" in var_context and isinstance(var_context["app"], App):
+        # which is a shiny.App object, the user probably made a mistake. (But if there's
+        # a magic comment to force it into Express mode, don't raise, because that means
+        # the user should know what they're doing.)
+        if (
+            "app" in var_context
+            and isinstance(var_context["app"], App)
+            and find_magic_comment_mode(content[:1000]) is None
+        ):
             raise RuntimeError(
                 "This looks like a Shiny Express app because it imports shiny.express, "
                 "but it also looks like a Shiny Core app because it has a variable named "

From 635a73f0b9ab08bc0c87232d2396cd2545f790fb Mon Sep 17 00:00:00 2001
From: Winston Chang <winston@posit.co>
Date: Wed, 13 Mar 2024 19:28:41 -0500
Subject: [PATCH 4/7] page_opts() and app_opt() raise errors if used outside of
 Express

---
 shiny/express/_run.py     | 15 +++++++++++++--
 shiny/express/ui/_page.py |  9 ++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/shiny/express/_run.py b/shiny/express/_run.py
index d272b11b7..7de634740 100644
--- a/shiny/express/_run.py
+++ b/shiny/express/_run.py
@@ -172,7 +172,7 @@ def set_result(x: object):
         sys.displayhook = prev_displayhook
 
 
-_top_level_recall_context_manager: RecallContextManager[Tag]
+_top_level_recall_context_manager: RecallContextManager[Tag] | None = None
 
 
 def reset_top_level_recall_context_manager() -> None:
@@ -183,6 +183,9 @@ def reset_top_level_recall_context_manager() -> None:
 
 
 def get_top_level_recall_context_manager() -> RecallContextManager[Tag]:
+    if _top_level_recall_context_manager is None:
+        raise RuntimeError("No top-level recall context manager has been set.")
+
     return _top_level_recall_context_manager
 
 
@@ -228,8 +231,16 @@ def app_opts(
         Whether to enable debug mode.
     """
 
-    # Store these options only if we're in the UI-rendering phase of Shiny Express.
     mock_session = get_current_session()
+
+    if mock_session is None:
+        # We can get here if a Shiny Core app, or if we're in the UI rendering phase of
+        # a Quarto-Shiny dashboard.
+        raise RuntimeError(
+            "express.app_opts() can only be used in a standalone Shiny Express app."
+        )
+
+    # Store these options only if we're in the UI-rendering phase of Shiny Express.
     if not isinstance(mock_session, ExpressMockSession):
         return
 
diff --git a/shiny/express/ui/_page.py b/shiny/express/ui/_page.py
index d005540e9..738d25356 100644
--- a/shiny/express/ui/_page.py
+++ b/shiny/express/ui/_page.py
@@ -58,7 +58,14 @@ def page_opts(
         one based on the arguments provided. If not ``None``, this will override all
         heuristics for choosing page functions.
     """
-    cm = get_top_level_recall_context_manager()
+    try:
+        cm = get_top_level_recall_context_manager()
+    except RuntimeError:
+        # We can get here if a Shiny Core app, or if we're in the UI rendering phase of
+        # a Quarto-Shiny dashboard.
+        raise RuntimeError(
+            "express.ui.page_opts() can only be used inside of a standalone Shiny Express app."
+        )
 
     if not isinstance(title, MISSING_TYPE):
         cm.kwargs["title"] = title

From a7043cdff74d863088cd86fb79fe66fc6abe4773 Mon Sep 17 00:00:00 2001
From: Winston Chang <winston@posit.co>
Date: Thu, 14 Mar 2024 15:06:32 -0500
Subject: [PATCH 5/7] Print message when shiny_mode is not express or core

---
 shiny/express/_is_express.py | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/shiny/express/_is_express.py b/shiny/express/_is_express.py
index 04ac4b378..68dd4e3dc 100644
--- a/shiny/express/_is_express.py
+++ b/shiny/express/_is_express.py
@@ -6,8 +6,9 @@
 
 import ast
 import re
+import sys
 from pathlib import Path
-from typing import Literal
+from typing import Literal, cast
 
 from .._docstring import no_example
 
@@ -95,17 +96,21 @@ def find_magic_comment_mode(content: str) -> Literal["core", "express"] | None:
     Look for a magic comment of the form "# shiny_mode: express" or "# shiny_mode:
     core".
 
+    If a line of the form "# shiny_mode: x" is found, where "x" is not "express" or
+    "core", then a message will be printed to stderr.
+
     Returns
     -------
     :
         `True` if Shiny Express comment is found, `False` if Shiny Core comment is
         found, and `None` if no magic comment is found.
     """
-
-    for line in content[:1000].splitlines():
-        if re.search(r"^#\s*shiny_mode:\s*express\s*$", line):
-            return "express"
-        if re.search(r"^#\s*shiny_mode:\s*core\s*$", line):
-            return "core"
+    m = re.search(r"^#[ \t]*shiny_mode:[ \t]*(\S*)[ \t]*$", content, re.MULTILINE)
+    if m is not None:
+        shiny_mode = cast(str, m.group(1))
+        if shiny_mode in ("express", "core"):
+            return shiny_mode
+        else:
+            print(f'Invalid shiny_mode: "{shiny_mode}"', file=sys.stderr)
 
     return None

From 17d8de5583daf67efed44f13139c9db87b950e10 Mon Sep 17 00:00:00 2001
From: Winston Chang <winston@posit.co>
Date: Thu, 14 Mar 2024 15:08:51 -0500
Subject: [PATCH 6/7] Read file as utf-8

---
 shiny/express/_is_express.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shiny/express/_is_express.py b/shiny/express/_is_express.py
index 68dd4e3dc..a62f0b408 100644
--- a/shiny/express/_is_express.py
+++ b/shiny/express/_is_express.py
@@ -45,7 +45,7 @@ def is_express_app(app: str, app_dir: str | None) -> bool:
 
     try:
         # Read the file, parse it, and look for any imports of shiny.express.
-        with open(app_path) as f:
+        with open(app_path, encoding="utf-8") as f:
             content = f.read()
 
         # Check for magic comment in the first 1000 characters

From 1f8f1ce18c9ca1afb9dbddbe7dea1113fbd857b5 Mon Sep 17 00:00:00 2001
From: Winston Chang <winston@posit.co>
Date: Thu, 14 Mar 2024 15:09:54 -0500
Subject: [PATCH 7/7] Update changelog

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 51cd06725..929c1b4c4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * `ui.card()` and `ui.value_box()` now take an `id` argument that, when provided, is used to report the full screen state of the card or value box to the server. For example, when using `ui.card(id = "my_card", full_screen = TRUE)` you can determine if the card is currently in full screen mode by reading the boolean value of `input.my_card()["full_screen"]`. (#1215)
 
+* Added support for using `shiny.express` in Quarto Dashboards. (#1217)
+
 ### Bug fixes
 
 ### Other changes