Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions pylings/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,21 @@ def main(argv: list[str] | None = None) -> int:
return _cmd_reset(args.root, args.name, args.yes)

if args.command in (None, "watch", "start", "topics"):
from pylings.core.curriculum import ensure_workspace
root, created = ensure_workspace(args.root)
if root != args.root.expanduser().resolve():
if created:
print(f"No pylings workspace found here; created one at {root}")
else:
print(f"Using existing pylings workspace at {root}")
Comment on lines +293 to +299

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle WorkspaceError from ensure_workspace gracefully.

ensure_workspace can raise WorkspaceError (e.g., a non-empty ./pylings-workspace without info.toml). The handler at lines 316-322 only converts ManifestError, so this surfaces as a raw traceback — the opposite of the friendly first-run behavior this PR targets.

🛡️ Proposed fix
         if args.command in (None, "watch", "start", "topics"):
-            from pylings.core.curriculum import ensure_workspace
-            root, created = ensure_workspace(args.root)
+            from pylings.core.curriculum import WorkspaceError, ensure_workspace
+            try:
+                root, created = ensure_workspace(args.root)
+            except WorkspaceError as e:
+                sys.stderr.write(f"pylings: {e}\n")
+                return 2
             if root != args.root.expanduser().resolve():
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pylings/cli.py` around lines 293 - 299, ensure_workspace can raise
WorkspaceError but the current code only converts ManifestError, so wrap the
call to ensure_workspace in a try/except that catches WorkspaceError (import
WorkspaceError from pylings.core.curriculum) and handle it gracefully: print a
concise, user-friendly message including the error details (e.g., "Workspace
appears invalid: {e}. Try removing or repairing the pylings-workspace or run
init") and exit with a non-zero status (sys.exit(1)); keep the existing
created/root logic for successful returns.

start_topic = getattr(args, "topic", None)
if start_topic is not None:
from pylings.core.manifest import load as load_manifest
if _resolve_topic(load_manifest(args.root), start_topic) is None:
if _resolve_topic(load_manifest(root), start_topic) is None:
return 2
from pylings.app import run_tui # lazy: Textual is heavy
return run_tui(
args.root,
root,
start_topic,
force_picker=args.command == "topics",
watch_files=getattr(args, "watch_files", False),
Expand Down
21 changes: 21 additions & 0 deletions pylings/core/curriculum.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class WorkspaceError(RuntimeError):


CURRICULUM_DIRS = ("exercises", "checks", "solutions")
DEFAULT_WORKSPACE_DIRNAME = "pylings-workspace"
GITIGNORE_LINES = [
".pylings/state.json",
".pylings_debug.log",
Expand Down Expand Up @@ -79,6 +80,26 @@ def init_workspace(path: Path, *, force: bool = False) -> Path:
return path


def ensure_workspace(root: Path) -> tuple[Path, bool]:
"""Return a usable workspace for `root`, creating one if needed.

- If `root` is already a workspace (has `info.toml`), use it as-is.
- Else if a previously auto-created `root/pylings-workspace` exists, reuse it.
- Otherwise initialize a fresh workspace at `root/pylings-workspace`.

Returns `(workspace_path, created_new)`.
"""
root = root.expanduser().resolve()
if (root / "info.toml").exists():
return root, False

default = root / DEFAULT_WORKSPACE_DIRNAME
if (default / "info.toml").exists():
return default.resolve(), False

return init_workspace(default), True
Comment on lines +96 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Edge case: non-empty pylings-workspace without info.toml raises WorkspaceError.

If root/pylings-workspace already exists and is non-empty but lacks info.toml (e.g., a partial/aborted init or a user-created dir), init_workspace(default) raises WorkspaceError ("already exists and is not empty"). This propagates uncaught through cli.main (which only handles ManifestError), reintroducing a crash on the very first-run path this PR aims to smooth. See the related comment in cli.py.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pylings/core/curriculum.py` around lines 96 - 100, The current call to
init_workspace(default) can raise WorkspaceError when root/pylings-workspace
exists and is non-empty but lacks info.toml; wrap the init_workspace(default)
call in a try/except for WorkspaceError and, if the directory already exists and
contains files (i.e., default.exists() and any(default.iterdir())), treat it as
an existing workspace by returning default.resolve(), False; otherwise re-raise
the WorkspaceError. Reference init_workspace and WorkspaceError so you can
locate and update the error handling around that call.



def update_workspace(path: Path) -> Path:
path = path.expanduser().resolve()
if not (path / "info.toml").exists():
Expand Down
29 changes: 25 additions & 4 deletions pylings/screens/track.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,17 @@
_DEBOUNCE_SECONDS = 0.6


def celebration_message(total: int) -> str:
"""Message shown when every exercise in the curriculum is complete."""
return (
f"🎉 You finished all {total} pylings exercises! 🎉\n\n"
"That's the whole curriculum — nicely done.\n"
f"Share it: \"I just completed all {total} pylings Python exercises 🎉\"\n"
"If pylings helped, a ⭐ on GitHub or a contribution is hugely appreciated.\n\n"
"Press Ctrl+Q to quit, or F4 to revisit topics."
)


class TrackScreen(Screen[None]):
"""One topic's linear track: editor + output + auto-save loop."""

Expand Down Expand Up @@ -82,7 +93,11 @@ def _initial_exercise(self) -> str | None:
def _render_state(self) -> None:
exs = self._exercises()
done = sum(1 for ex in exs if ex.name in self.app.state.completed)
self.query_one(ProgressBar).update_progress(done, len(exs))
overall_total = len(self.app.manifest.exercises)
overall_done = len(self.app.state.completed)
self.query_one(ProgressBar).update_progress(
done, len(exs), overall_done, overall_total
)
self.query_one(ExerciseTree).render_topic(
self.topic, exs, self.app.state.completed, self.current
)
Expand Down Expand Up @@ -175,9 +190,15 @@ def _apply_result(self, exercise: Exercise, result: RunResult) -> None:
self._render_state()
if self.current is None:
self._record_resume(None)
self.query_one(OutputPanel).show_final(
f"Topic '{self.topic}' complete — press F4 for topics."
)
all_exercises = self.app.manifest.exercises
if next_pending(all_exercises, self.app.state.completed) is None:
self.query_one(OutputPanel).show_final(
celebration_message(len(all_exercises))
)
else:
self.query_one(OutputPanel).show_final(
f"Topic '{self.topic}' complete — press F4 for topics."
)
return
self._load_current()
self._run_current()
Expand Down
36 changes: 30 additions & 6 deletions pylings/widgets/progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,37 @@

from textual.widgets import Static

_BAR_WIDTH = 20


def _bar(completed: int, total: int) -> tuple[str, float]:
pct = (completed / total * 100) if total else 0
filled = int(_BAR_WIDTH * pct / 100)
return "█" * filled + "░" * (_BAR_WIDTH - filled), pct


def format_progress(
completed: int, total: int, overall_completed: int, overall_total: int
) -> str:
"""Render a one-line progress display showing topic and overall progress."""
topic_bar, topic_pct = _bar(completed, total)
overall_bar, overall_pct = _bar(overall_completed, overall_total)
return (
f"Topic: {topic_bar} {completed}/{total} ({topic_pct:.0f}%)"
f" Overall: {overall_bar} {overall_completed}/{overall_total} ({overall_pct:.0f}%)"
)


class ProgressBar(Static):
DEFAULT_CSS = ""

def update_progress(self, completed: int, total: int) -> None:
pct = (completed / total * 100) if total else 0
bar_width = 20
filled = int(bar_width * pct / 100)
bar = "█" * filled + "░" * (bar_width - filled)
self.update(f"Progress: {bar} {completed}/{total} ({pct:.0f}%)")
def update_progress(
self,
completed: int,
total: int,
overall_completed: int,
overall_total: int,
) -> None:
self.update(
format_progress(completed, total, overall_completed, overall_total)
)
21 changes: 21 additions & 0 deletions tests/integration/test_cli_autoinit.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from __future__ import annotations

import subprocess
import sys
from pathlib import Path


def _run(*args: str) -> subprocess.CompletedProcess[str]:
return subprocess.run(
[sys.executable, "-m", "pylings", *args],
capture_output=True,
text=True,
)


def test_launch_in_empty_dir_autoinits_workspace(tmp_path: Path) -> None:
# `start` with a non-existent topic returns before launching the TUI, so we
# can assert the auto-init side effect without needing a terminal.
_run("--root", str(tmp_path), "start", "__no_such_topic__")

assert (tmp_path / "pylings-workspace" / "info.toml").exists()
10 changes: 10 additions & 0 deletions tests/unit/test_celebration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from __future__ import annotations

from pylings.screens.track import celebration_message


def test_celebration_message_includes_count_and_is_celebratory() -> None:
msg = celebration_message(292)

assert "292" in msg
assert "🎉" in msg
22 changes: 22 additions & 0 deletions tests/unit/test_progress.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from __future__ import annotations

from pylings.widgets.progress import format_progress


def test_format_progress_shows_topic_and_overall() -> None:
line = format_progress(2, 5, 10, 100)

# topic portion: 2/5 == 40%
assert "2/5" in line
assert "40%" in line
# overall portion: 10/100 == 10%
assert "10/100" in line
assert "10%" in line
assert "Overall" in line


def test_format_progress_handles_zero_totals() -> None:
line = format_progress(0, 0, 0, 0)

assert "0/0" in line
assert "0%" in line
31 changes: 31 additions & 0 deletions tests/unit/test_workspace_autoinit.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
from __future__ import annotations

from pathlib import Path

from pylings.core.curriculum import ensure_workspace, init_workspace


def test_autoinits_workspace_in_empty_dir(tmp_path: Path) -> None:
work, created = ensure_workspace(tmp_path)

assert created is True
assert work == (tmp_path / "pylings-workspace").resolve()
assert (work / "info.toml").exists()


def test_returns_root_unchanged_when_already_a_workspace(tmp_path: Path) -> None:
init_workspace(tmp_path)

work, created = ensure_workspace(tmp_path)

assert created is False
assert work == tmp_path.resolve()


def test_reuses_previously_autoinited_workspace(tmp_path: Path) -> None:
first, first_created = ensure_workspace(tmp_path)
second, second_created = ensure_workspace(tmp_path)

assert first_created is True
assert second_created is False
assert second == first
Loading