diff --git a/frontend/src/components/app-config/app-config-button.tsx b/frontend/src/components/app-config/app-config-button.tsx index 662aeeffdc0..a2b4ea37e59 100644 --- a/frontend/src/components/app-config/app-config-button.tsx +++ b/frontend/src/components/app-config/app-config-button.tsx @@ -10,15 +10,14 @@ import { UserConfigForm } from "./user-config-form"; import { Tooltip } from "../ui/tooltip"; import { Dialog, DialogTrigger, DialogContent } from "../ui/dialog"; import { AppConfigForm } from "@/components/app-config/app-config-form"; -import { atom, useAtom } from "jotai"; +import { useAtom } from "jotai"; import { Button } from "../ui/button"; +import { settingDialogAtom } from "./state"; interface Props { showAppConfig?: boolean; } -export const settingDialogAtom = atom(false); - export const ConfigButton: React.FC = ({ showAppConfig = true }) => { const [settingDialog, setSettingDialog] = useAtom(settingDialogAtom); const button = ( diff --git a/frontend/src/components/app-config/state.ts b/frontend/src/components/app-config/state.ts new file mode 100644 index 00000000000..88d5147d4c4 --- /dev/null +++ b/frontend/src/components/app-config/state.ts @@ -0,0 +1,18 @@ +/* Copyright 2024 Marimo. All rights reserved. */ +import { atom, useSetAtom } from "jotai"; +import { + activeUserConfigCategoryAtom, + type SettingCategoryId, +} from "./user-config-form"; + +export const settingDialogAtom = atom(false); + +export function useOpenSettingsToTab() { + const setActiveCategory = useSetAtom(activeUserConfigCategoryAtom); + const setSettingsDialog = useSetAtom(settingDialogAtom); + const handleClick = (tab: SettingCategoryId) => { + setActiveCategory(tab); + setSettingsDialog(true); + }; + return { handleClick }; +} diff --git a/frontend/src/components/app-config/user-config-form.tsx b/frontend/src/components/app-config/user-config-form.tsx index 173684bd437..628888cad63 100644 --- a/frontend/src/components/app-config/user-config-form.tsx +++ b/frontend/src/components/app-config/user-config-form.tsx @@ -90,9 +90,11 @@ const categories = [ }, ] as const; -type CategoryId = (typeof categories)[number]["id"]; +export type SettingCategoryId = (typeof categories)[number]["id"]; -export const activeUserConfigCategoryAtom = atom(categories[0].id); +export const activeUserConfigCategoryAtom = atom( + categories[0].id, +); export const UserConfigForm: React.FC = () => { const [config, setConfig] = useUserConfig(); @@ -1137,11 +1139,13 @@ export const UserConfigForm: React.FC = () => { > setActiveCategory(value as CategoryId)} + onValueChange={(value) => + setActiveCategory(value as SettingCategoryId) + } orientation="vertical" className="w-1/3 pr-4 border-r h-full overflow-auto p-6" > - + {categories.map((category) => ( { event?.preventDefault(); diff --git a/frontend/src/components/editor/chrome/panels/packages-panel.tsx b/frontend/src/components/editor/chrome/panels/packages-panel.tsx index b7b118bb0b4..53513d62e4c 100644 --- a/frontend/src/components/editor/chrome/panels/packages-panel.tsx +++ b/frontend/src/components/editor/chrome/panels/packages-panel.tsx @@ -28,6 +28,7 @@ import { Kbd } from "@/components/ui/kbd"; import { Events } from "@/utils/events"; import { copyToClipboard } from "@/utils/copy"; import { PACKAGES_INPUT_ID } from "./constants"; +import { useOpenSettingsToTab } from "@/components/app-config/state"; export const PackagesPanel: React.FC = () => { const [config] = useResolvedMarimoConfig(); @@ -62,6 +63,7 @@ const InstallPackageForm: React.FC<{ }> = ({ onSuccess, packageManager }) => { const [input, setInput] = React.useState(""); const [loading, setLoading] = React.useState(false); + const { handleClick: openSettings } = useOpenSettingsToTab(); const handleAddPackage = async () => { try { @@ -103,7 +105,12 @@ const InstallPackageForm: React.FC<{ className="mr-2 h-4 w-4 shrink-0 opacity-50" /> ) : ( - + + openSettings("packageManagement")} + className="mr-2 h-4 w-4 shrink-0 opacity-50 hover:opacity-80 cursor-pointer" + /> + ) } rootClassName="flex-1 border-none" diff --git a/frontend/src/components/editor/chrome/wrapper/copilot-status.tsx b/frontend/src/components/editor/chrome/wrapper/copilot-status.tsx index 279c82bb9db..fa7441de84b 100644 --- a/frontend/src/components/editor/chrome/wrapper/copilot-status.tsx +++ b/frontend/src/components/editor/chrome/wrapper/copilot-status.tsx @@ -12,25 +12,24 @@ import { aiEnabledAtom, resolvedMarimoConfigAtom } from "@/core/config/config"; import { GitHubCopilotIcon } from "@/components/icons/github-copilot"; import { SparklesIcon } from "lucide-react"; import { FooterItem } from "./footer-item"; -import { activeUserConfigCategoryAtom } from "@/components/app-config/user-config-form"; -import { settingDialogAtom } from "@/components/app-config/app-config-button"; import { toast } from "@/components/ui/use-toast"; import { getCopilotClient } from "@/core/codemirror/copilot/client"; import { Logger } from "@/utils/Logger"; import { Button } from "@/components/ui/button"; import { useOnMount } from "@/hooks/useLifecycle"; +import { useOpenSettingsToTab } from "@/components/app-config/state"; export const AIStatusIcon: React.FC = () => { const ai = useAtomValue(aiAtom); const aiEnabled = useAtomValue(aiEnabledAtom); const model = ai?.open_ai?.model || "gpt-4-turbo"; - const { handleClick } = useOpenAISettings(); + const { handleClick } = useOpenSettingsToTab(); if (!aiEnabled) { return ( handleClick("ai")} > @@ -44,7 +43,7 @@ export const AIStatusIcon: React.FC = () => { Assist model: {model} } - onClick={handleClick} + onClick={() => handleClick("ai")} selected={false} > @@ -59,16 +58,6 @@ const aiAtom = atom((get) => { return get(resolvedMarimoConfigAtom).ai; }); -export function useOpenAISettings() { - const setActiveCategory = useSetAtom(activeUserConfigCategoryAtom); - const setSettingsDialog = useSetAtom(settingDialogAtom); - const handleClick = () => { - setActiveCategory("ai"); - setSettingsDialog(true); - }; - return { handleClick }; -} - export const CopilotStatusIcon: React.FC = () => { const copilot = useAtomValue(copilotAtom); @@ -84,7 +73,8 @@ export const CopilotStatusIcon: React.FC = () => { const GitHubCopilotStatus: React.FC = () => { const isGitHubCopilotSignedIn = useAtomValue(isGitHubCopilotSignedInState); const isLoading = useAtomValue(githubCopilotLoadingVersion) !== null; - const { handleClick } = useOpenAISettings(); + const { handleClick } = useOpenSettingsToTab(); + const openSettings = () => handleClick("ai"); const label = isGitHubCopilotSignedIn ? "Ready" : "Not connected"; const setCopilotSignedIn = useSetAtom(isGitHubCopilotSignedInState); @@ -128,7 +118,7 @@ const GitHubCopilotStatus: React.FC = () => { "Failed to connect to GitHub Copilot. Check settings and try again.", variant: "danger", action: ( - ), @@ -151,7 +141,7 @@ const GitHubCopilotStatus: React.FC = () => { } selected={false} - onClick={handleClick} + onClick={openSettings} > {isLoading ? ( diff --git a/frontend/src/core/codemirror/copilot/copilot-config.tsx b/frontend/src/core/codemirror/copilot/copilot-config.tsx index 13f7c044dac..6965d9b125b 100644 --- a/frontend/src/core/codemirror/copilot/copilot-config.tsx +++ b/frontend/src/core/codemirror/copilot/copilot-config.tsx @@ -4,19 +4,19 @@ import { copilotSignedInState, isGitHubCopilotSignedInState } from "./state"; import { memo, useState } from "react"; import { getCopilotClient } from "./client"; import { Button, buttonVariants } from "@/components/ui/button"; -import { useOpenAISettings } from "@/components/editor/chrome/wrapper/copilot-status"; import { CheckIcon, CopyIcon, Loader2Icon, XIcon } from "lucide-react"; import { Label } from "@/components/ui/label"; import { toast } from "@/components/ui/use-toast"; import { copyToClipboard } from "@/utils/copy"; import { Logger } from "@/utils/Logger"; +import { useOpenSettingsToTab } from "@/components/app-config/state"; export const CopilotConfig = memo(() => { const [copilotSignedIn, copilotChangeSignIn] = useAtom( isGitHubCopilotSignedInState, ); const [step, setStep] = useAtom(copilotSignedInState); - const { handleClick: openSettings } = useOpenAISettings(); + const { handleClick: openSettings } = useOpenSettingsToTab(); const [localData, setLocalData] = useState<{ url: string; code: string }>(); const [loading, setLoading] = useState(false); @@ -125,7 +125,7 @@ export const CopilotConfig = memo(() => { "Lost connection during sign-in. Please check settings and try again.", variant: "danger", action: ( - ), diff --git a/marimo/_config/packages.py b/marimo/_config/packages.py index 93ae956534f..71acb7f01ff 100644 --- a/marimo/_config/packages.py +++ b/marimo/_config/packages.py @@ -4,10 +4,14 @@ import os import sys from pathlib import Path -from typing import Literal +from typing import Literal, Optional +from marimo._config.utils import read_toml -def infer_package_manager() -> Literal["pip", "rye", "uv", "poetry", "pixi"]: +PackageManagerKind = Literal["pip", "rye", "uv", "poetry", "pixi"] + + +def infer_package_manager() -> PackageManagerKind: """Infer the package manager from the current project.""" try: @@ -22,18 +26,25 @@ def infer_package_manager() -> Literal["pip", "rye", "uv", "poetry", "pixi"]: break root_dir = root_dir.parent - # Check for Poetry - if (root_dir / "poetry.lock").exists(): - return "poetry" + # If there is a pyproject.toml, try to infer the package manager + pyproject_toml = root_dir / "pyproject.toml" + if pyproject_toml.exists(): + package_manager = infer_package_manager_from_pyproject( + pyproject_toml + ) + if package_manager is not None: + return package_manager - # Check for Rye - if (root_dir / ".rye").exists(): - return "rye" + # Try to infer from lockfiles + package_manager = infer_package_manager_from_lockfile(root_dir) + if package_manager is not None: + return package_manager - # Check for Pixi + # misc - Check for pixi.toml if (root_dir / "pixi.toml").exists(): return "pixi" + # misc - Check for virtualenv/pip VIRTUAL_ENV = os.environ.get("VIRTUAL_ENV", "") # Check for '/uv/' in VIRTUAL_ENV @@ -51,3 +62,49 @@ def infer_package_manager() -> Literal["pip", "rye", "uv", "poetry", "pixi"]: except Exception: # Fallback to pip return "pip" + + +def infer_package_manager_from_pyproject( + pyproject_toml: Path, +) -> Optional[PackageManagerKind]: + """Infer the package manager from a pyproject.toml file.""" + try: + data = read_toml(pyproject_toml) + + if "tool" not in data: + return None + + to_check: list[PackageManagerKind] = [ + "poetry", + "pixi", + "uv", + "rye", + ] + + for manager in to_check: + if manager in data["tool"]: + return manager + + return None + except Exception: + # Fallback to None + return None + + +def infer_package_manager_from_lockfile( + root_dir: Path, +) -> Optional[PackageManagerKind]: + """Infer the package manager from a lockfile.""" + lockfile_map: dict[str, PackageManagerKind] = { + "poetry.lock": "poetry", + "pixi.lock": "pixi", + ".uv": "uv", + "requirements.lock": "rye", + } + try: + for lockfile, manager in lockfile_map.items(): + if (root_dir / lockfile).exists(): + return manager + return None + except Exception: + return None diff --git a/marimo/_config/reader.py b/marimo/_config/reader.py index 822421789ce..675cd3f412c 100644 --- a/marimo/_config/reader.py +++ b/marimo/_config/reader.py @@ -1,21 +1,14 @@ # Copyright 2024 Marimo. All rights reserved. from pathlib import Path -from typing import Any, Dict, Optional, Union, cast +from typing import Optional, Union, cast from marimo import _loggers from marimo._config.config import PartialMarimoConfig +from marimo._config.utils import read_toml LOGGER = _loggers.marimo_logger() -def read_toml(file_path: Union[str, Path]) -> Dict[str, Any]: - """Read and parse a TOML file.""" - import tomlkit - - with open(file_path, "rb") as file: - return tomlkit.load(file) - - def read_marimo_config(path: str) -> PartialMarimoConfig: """Read the marimo.toml configuration.""" return cast(PartialMarimoConfig, read_toml(path)) diff --git a/marimo/_config/utils.py b/marimo/_config/utils.py index 699d6a879bf..432879d89d1 100644 --- a/marimo/_config/utils.py +++ b/marimo/_config/utils.py @@ -2,15 +2,26 @@ from __future__ import annotations import os -from typing import Any, Optional +from typing import TYPE_CHECKING, Any, Dict, Optional, Union from marimo import _loggers +if TYPE_CHECKING: + from pathlib import Path + LOGGER = _loggers.marimo_logger() CONFIG_FILENAME = ".marimo.toml" +def read_toml(file_path: Union[str, Path]) -> Dict[str, Any]: + """Read and parse a TOML file.""" + import tomlkit + + with open(file_path, "rb") as file: + return tomlkit.load(file) + + def _is_parent(parent_path: str, child_path: str) -> bool: # Check if parent is actually a parent of child # paths must be real/absolute paths diff --git a/marimo/_utils/config/config.py b/marimo/_utils/config/config.py index 54f6c63e773..39acbe3ca6e 100644 --- a/marimo/_utils/config/config.py +++ b/marimo/_utils/config/config.py @@ -6,6 +6,7 @@ from tempfile import TemporaryDirectory from typing import Any, Optional, Type, TypeVar +from marimo._config.utils import read_toml from marimo._utils.parse_dataclass import parse_raw ROOT_DIR = ".marimo" @@ -33,9 +34,8 @@ def read_toml(self, cls: Type[T], *, fallback: T) -> T: import tomlkit try: - with open(self.filepath, "r") as file: - data = tomlkit.parse(file.read()) - return parse_raw(data, cls, allow_unknown_keys=True) + data = read_toml(self.filepath) + return parse_raw(data, cls, allow_unknown_keys=True) except (FileNotFoundError, tomlkit.exceptions.TOMLKitError): return fallback diff --git a/tests/_config/test_config_packages.py b/tests/_config/test_config_packages.py new file mode 100644 index 00000000000..6548a2b8598 --- /dev/null +++ b/tests/_config/test_config_packages.py @@ -0,0 +1,130 @@ +from __future__ import annotations + +import os +import sys +from pathlib import Path +from typing import Any +from unittest.mock import patch + +import pytest + +from marimo._config.packages import ( + infer_package_manager, + infer_package_manager_from_lockfile, + infer_package_manager_from_pyproject, +) + + +@pytest.fixture +def mock_cwd(tmp_path: Path): + """Creates a temporary directory and sets it as CWD""" + old_cwd = os.getcwd() + os.chdir(tmp_path) + yield tmp_path + os.chdir(old_cwd) + + +def test_infer_package_manager_from_pyproject(): + # Test poetry detection + with patch( + "marimo._config.packages.read_toml", + return_value={"tool": {"poetry": {}}}, + ): + assert ( + infer_package_manager_from_pyproject(Path("pyproject.toml")) + == "poetry" + ) + + # Test no tool section + with patch("marimo._config.packages.read_toml", return_value={}): + assert ( + infer_package_manager_from_pyproject(Path("pyproject.toml")) + is None + ) + + # Test exception handling + with patch("marimo._config.packages.read_toml", side_effect=Exception): + assert ( + infer_package_manager_from_pyproject(Path("pyproject.toml")) + is None + ) + + +def test_infer_package_manager_from_lockfile(mock_cwd: Path): + # Test poetry.lock + (mock_cwd / "poetry.lock").touch() + assert infer_package_manager_from_lockfile(mock_cwd) == "poetry" + (mock_cwd / "poetry.lock").unlink() + + # Test pixi.lock + (mock_cwd / "pixi.lock").touch() + assert infer_package_manager_from_lockfile(mock_cwd) == "pixi" + (mock_cwd / "pixi.lock").unlink() + + # Test no lockfile + for f in mock_cwd.iterdir(): + f.unlink() + assert infer_package_manager_from_lockfile(mock_cwd) is None + + +TEST_CASES: list[ + tuple[dict[str, Any], dict[str, Any], dict[str, Any], str] +] = [ + # Test pyproject.toml with poetry + ({"pyproject.toml": {"tool": {"poetry": {}}}}, {}, {}, "poetry"), + # Test lockfile detection + ({"poetry.lock": ""}, {}, {}, "poetry"), + # Test pixi.toml + ({"pixi.toml": ""}, {}, {}, "pixi"), + # Test fallback to pip + ({}, {}, {}, "pip"), +] + +if sys.platform != "win32": + TEST_CASES.extend( + [ + # Test uv virtualenv + ({}, {"VIRTUAL_ENV": "/path/uv/env"}, {}, "uv"), + # Test regular virtualenv + ({}, {}, {"base_prefix": "/usr", "prefix": "/venv"}, "pip"), + ] + ) + + +@pytest.mark.parametrize( + ("files", "env_vars", "sys_attrs", "expected"), + TEST_CASES, +) +def test_infer_package_manager( + mock_cwd: Path, + files: dict[str, Any], + env_vars: dict[str, Any], + sys_attrs: dict[str, Any], + expected: str, +): + # Write a default pyproject.toml file + (mock_cwd / "pyproject.toml").write_text( + """ + [project] + name = "test" + """ + ) + + # Create test files + for filename, content in files.items(): + if isinstance(content, dict): + import tomlkit + + with open(mock_cwd / filename, "w") as f: + tomlkit.dump(content, f) + else: + (mock_cwd / filename).write_text(content) + + # Mock environment variables + with patch.dict(os.environ, env_vars): + # Mock sys attributes + if sys_attrs: + with patch.multiple(sys, **sys_attrs): + assert infer_package_manager() == expected + else: + assert infer_package_manager() == expected diff --git a/tests/_server/templates/snapshots/export1.txt b/tests/_server/templates/snapshots/export1.txt index 9c3a7004ece..1c9ad323209 100644 --- a/tests/_server/templates/snapshots/export1.txt +++ b/tests/_server/templates/snapshots/export1.txt @@ -55,7 +55,7 @@ - + notebook diff --git a/tests/_server/templates/snapshots/export2.txt b/tests/_server/templates/snapshots/export2.txt index 993de9ab271..21e76164f78 100644 --- a/tests/_server/templates/snapshots/export2.txt +++ b/tests/_server/templates/snapshots/export2.txt @@ -55,7 +55,7 @@ - + marimo diff --git a/tests/_server/templates/snapshots/export3.txt b/tests/_server/templates/snapshots/export3.txt index bd251d193c7..bb2f00d7ab0 100644 --- a/tests/_server/templates/snapshots/export3.txt +++ b/tests/_server/templates/snapshots/export3.txt @@ -55,7 +55,7 @@ - + notebook diff --git a/tests/_server/templates/snapshots/export4.txt b/tests/_server/templates/snapshots/export4.txt index ff2d329754e..cc12f97fce6 100644 --- a/tests/_server/templates/snapshots/export4.txt +++ b/tests/_server/templates/snapshots/export4.txt @@ -55,7 +55,7 @@ - + notebook diff --git a/tests/_server/templates/snapshots/export5.txt b/tests/_server/templates/snapshots/export5.txt index 0761bc7a4a9..ba7f3bfb95f 100644 --- a/tests/_server/templates/snapshots/export5.txt +++ b/tests/_server/templates/snapshots/export5.txt @@ -55,7 +55,7 @@ - + notebook