Skip to content

Commit

Permalink
improvement: click package icon to change the package manager (#3691)
Browse files Browse the repository at this point in the history
Make the package icon in the package manager clickable. 
Improve the infer_package_manager and add tests.
  • Loading branch information
mscolnick authored Feb 6, 2025
1 parent 7f3ec27 commit e40879e
Show file tree
Hide file tree
Showing 17 changed files with 266 additions and 57 deletions.
5 changes: 2 additions & 3 deletions frontend/src/components/app-config/app-config-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>(false);

export const ConfigButton: React.FC<Props> = ({ showAppConfig = true }) => {
const [settingDialog, setSettingDialog] = useAtom(settingDialogAtom);
const button = (
Expand Down
18 changes: 18 additions & 0 deletions frontend/src/components/app-config/state.ts
Original file line number Diff line number Diff line change
@@ -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<boolean>(false);

export function useOpenSettingsToTab() {
const setActiveCategory = useSetAtom(activeUserConfigCategoryAtom);
const setSettingsDialog = useSetAtom(settingDialogAtom);
const handleClick = (tab: SettingCategoryId) => {
setActiveCategory(tab);
setSettingsDialog(true);
};
return { handleClick };
}
12 changes: 8 additions & 4 deletions frontend/src/components/app-config/user-config-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<CategoryId>(categories[0].id);
export const activeUserConfigCategoryAtom = atom<SettingCategoryId>(
categories[0].id,
);

export const UserConfigForm: React.FC = () => {
const [config, setConfig] = useUserConfig();
Expand Down Expand Up @@ -1161,11 +1163,13 @@ export const UserConfigForm: React.FC = () => {
>
<Tabs
value={activeCategory}
onValueChange={(value) => 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"
>
<TabsList className="self-start max-h-none flex flex-col gap-2 flex-shrink-0 bg-background flex-1 min-h-full">
<TabsList className="self-start max-h-none flex flex-col gap-2 shrink-0 bg-background flex-1 min-h-full">
{categories.map((category) => (
<TabsTrigger
key={category.id}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ import { useLayoutState, useLayoutActions } from "@/core/layout/layout";
import { useTogglePresenting } from "@/core/layout/useTogglePresenting";
import { useCopyNotebook } from "./useCopyNotebook";
import { isWasm } from "@/core/wasm/utils";
import { settingDialogAtom } from "@/components/app-config/app-config-button";
import { renderShortcut } from "@/components/shortcuts/renderShortcut";
import { copyToClipboard } from "@/utils/copy";
import { newNotebookURL } from "@/utils/urls";
import { useRunAllCells } from "../cell/useRunCells";
import { settingDialogAtom } from "@/components/app-config/state";

const NOOP_HANDLER = (event?: Event) => {
event?.preventDefault();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -103,7 +105,12 @@ const InstallPackageForm: React.FC<{
className="mr-2 h-4 w-4 shrink-0 opacity-50"
/>
) : (
<BoxIcon className="mr-2 h-4 w-4 shrink-0 opacity-50" />
<Tooltip content="Change package manager">
<BoxIcon
onClick={() => openSettings("packageManagement")}
className="mr-2 h-4 w-4 shrink-0 opacity-50 hover:opacity-80 cursor-pointer"
/>
</Tooltip>
)
}
rootClassName="flex-1 border-none"
Expand Down
26 changes: 8 additions & 18 deletions frontend/src/components/editor/chrome/wrapper/copilot-status.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<FooterItem
tooltip="Assist is disabled"
selected={false}
onClick={handleClick}
onClick={() => handleClick("ai")}
>
<SparklesIcon className="h-4 w-4 opacity-60" />
</FooterItem>
Expand All @@ -44,7 +43,7 @@ export const AIStatusIcon: React.FC = () => {
<b>Assist model:</b> {model}
</>
}
onClick={handleClick}
onClick={() => handleClick("ai")}
selected={false}
>
<SparklesIcon className="h-4 w-4" />
Expand All @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -128,7 +118,7 @@ const GitHubCopilotStatus: React.FC = () => {
"Failed to connect to GitHub Copilot. Check settings and try again.",
variant: "danger",
action: (
<Button variant="link" onClick={handleClick}>
<Button variant="link" onClick={openSettings}>
Settings
</Button>
),
Expand All @@ -151,7 +141,7 @@ const GitHubCopilotStatus: React.FC = () => {
</>
}
selected={false}
onClick={handleClick}
onClick={openSettings}
>
<span>
{isLoading ? (
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/core/codemirror/copilot/copilot-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -125,7 +125,7 @@ export const CopilotConfig = memo(() => {
"Lost connection during sign-in. Please check settings and try again.",
variant: "danger",
action: (
<Button variant="link" onClick={openSettings}>
<Button variant="link" onClick={() => openSettings("ai")}>
Settings
</Button>
),
Expand Down
75 changes: 66 additions & 9 deletions marimo/_config/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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
11 changes: 2 additions & 9 deletions marimo/_config/reader.py
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
13 changes: 12 additions & 1 deletion marimo/_config/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions marimo/_utils/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down
Loading

0 comments on commit e40879e

Please sign in to comment.