test: Add simple happypath annotation adding

Editor function takes an optional io object which is used to print an
error output if the subprocess errors.

test/conftest.py

@@ -0,0 +1,27 @@
+from pathlib import Path
+
+import pytest
+
+from topen import OPTIONS
+
+
+@pytest.fixture
+def fake_id(monkeypatch):
+    monkeypatch.setattr("sys.argv", ["topen", "0"])
+
+
+@pytest.fixture
+def isolate_env(monkeypatch):
+    # delete all existing env vars that could interfere
+    monkeypatch.delenv("EDITOR", raising=False)
+    monkeypatch.delenv("VISUAL", raising=False)
+    for opt in OPTIONS.values():
+        if opt.env:
+            monkeypatch.delenv(opt.env, raising=False)
+
+
+@pytest.fixture
+def fake_rc(tmp_path: Path, monkeypatch):
+    rc = tmp_path / "test.taskrc"
+    monkeypatch.setattr(OPTIONS["task_rc"], "default", rc)
+    return rc

test/test_cli.py

@@ -0,0 +1,17 @@
+from pathlib import Path
+from unittest.mock import Mock, patch
+
+from topen import add_annotation, open_editor
+
+
+def test_open_editor_escapes_shell():
+    """Ensure filenames with spaces/metas do not allow shell injection."""
+    with patch("subprocess.run") as run_mock:
+        open_editor(Path("my note$1.txt"), "vim")
+    run_mock.assert_called_once_with(["vim", "my note$1.txt"], check=True)
+
+
+def test_add_annotation_calls_tasklib():
+    task = Mock()
+    add_annotation(task, "hello")
+    task.add_annotation.assert_called_once_with("hello")

test/test_configuration.py

@@ -1,118 +1,11 @@
+# pyright: reportUnusedImport=false, reportUnusedParameter=false
+# ruff: noqa: F401, F811
+# ^ Turn off for implicit pytest fixture import
 from pathlib import Path
 
 import pytest
 
-from topen import OPTIONS, TConf, parse_cli, parse_env, parse_rc
+from topen import TConf, build_config
-
-
-class TestCli:
-    def test_cli_minimum_id(self, monkeypatch):
-        monkeypatch.setattr("sys.argv", ["topen", "42"])
-        assert parse_cli() == {"task_id": "42"}
-
-    def test_cli_options(self, monkeypatch):
-        monkeypatch.setattr(
-            "sys.argv",
-            [
-                "topen",
-                "123",
-                "--extension",
-                "txt",
-                "--editor",
-                "vim",
-                "--quiet",
-                "True",
-            ],
-        )
-        assert parse_cli() == {
-            "task_id": "123",
-            "notes_ext": "txt",
-            "notes_editor": "vim",
-            "notes_quiet": True,
-        }
-
-
-@pytest.fixture
-def isolate_env(monkeypatch):
-    # delete all existing env vars that could interfere
-    monkeypatch.delenv("EDITOR", raising=False)
-    monkeypatch.delenv("VISUAL", raising=False)
-    for opt in OPTIONS.values():
-        if opt.env:
-            monkeypatch.delenv(opt.env, raising=False)
-
-
-class TestEnv:
-    def test_env_notes_ext(self, isolate_env, monkeypatch):
-        monkeypatch.setenv("TOPEN_NOTES_DIR", "/blablubb")
-        monkeypatch.setenv("TOPEN_NOTES_EXT", "rst")
-        monkeypatch.setenv("TOPEN_NOTES_ANNOT", "qmd")
-        monkeypatch.setenv("TOPEN_NOTES_EDITOR", "vim")
-        monkeypatch.setenv("TOPEN_NOTES_QUIET", "true")
-        env = parse_env()
-        assert env["notes_dir"] == Path("/blablubb")
-        assert env["notes_ext"] == "rst"
-        assert env["notes_annot"] == "qmd"
-        assert env["notes_editor"] == "vim"
-        assert env["notes_quiet"] is True
-
-    def test_env_task_rc(self, isolate_env, monkeypatch):
-        monkeypatch.setenv("TASKRC", "/a/dir/that/dont/exist/file")
-        monkeypatch.setenv("TASKDATA", "~/somewhere/tasks")
-        env = parse_env()
-        assert env["task_rc"] == Path("/a/dir/that/dont/exist/file")
-        assert env["task_data"] == Path("~/somewhere/tasks")
-
-    def test_env_parses_boolean_true(self, isolate_env, monkeypatch):
-        monkeypatch.setenv("TOPEN_NOTES_QUIET", "true")
-        env = parse_env()
-        assert env["notes_quiet"] is True
-
-    def test_env_parses_boolean_false(self, isolate_env, monkeypatch):
-        monkeypatch.setenv("TOPEN_NOTES_QUIET", "false")
-        env = parse_env()
-        assert env["notes_quiet"] is False
-
-
-@pytest.fixture
-def fake_rc(tmp_path: Path, monkeypatch):
-    rc = tmp_path / "test.taskrc"
-    monkeypatch.setattr(OPTIONS["task_rc"], "default", rc)
-    return rc
-
-
-class TestRcFile:
-    def test_taskrc_parsing(self, fake_rc):
-        fake_rc.write_text("""
-        data.location=~/.taskies
-        notes.dir=/there
-        notes.ext=yaml
-        notes.annot=Boo!
-        notes.editor=micro
-        notes.quiet=true
-        """)
-        rc_cfg = parse_rc(fake_rc)
-        assert rc_cfg["task_data"] == Path("~/.taskies")
-        assert rc_cfg["notes_dir"] == Path("/there")
-        assert rc_cfg["notes_ext"] == "yaml"
-        assert rc_cfg["notes_annot"] == "Boo!"
-        assert rc_cfg["notes_editor"] == "micro"
-        assert rc_cfg["notes_quiet"] is True
-
-    def test_taskrc_parses_boolean_true(self, fake_rc):
-        fake_rc.write_text("""
-        notes.quiet=true
-        """)
-        rc_cfg = parse_rc(fake_rc)
-        assert rc_cfg["notes_quiet"] is True
-
-    def test_taskrc_parses_boolean_false(self, fake_rc):
-        fake_rc.write_text("""
-        notes.quiet=false
-        """)
-        rc_cfg = parse_rc(fake_rc)
-        assert rc_cfg["notes_quiet"] is False
-
 
 class TestTConf:
     def test_paths_are_expanded(self):
@@ -140,7 +33,68 @@ class TestTConf:
             ({"VISUAL": "nvim", "EDITOR": "notepad"}, "notepad"),
         ],
     )
-    def test_editor_env_resolution(isolate_env, monkeypatch, env, expected):
+    def test_editor_env_resolution(self, isolate_env, monkeypatch, env, expected):
         for k, v in env.items():
             monkeypatch.setenv(k, v)
         assert TConf(0).notes_editor == expected
+
+
+class TestBuildConfigPrecedence:
+    """
+    All tests exercise the same key (notes_ext) to keep the assertions short.
+    Each source sets a different value so we can be sure the right one wins.
+    """
+
+    def test_defaults_only(self, fake_rc, monkeypatch, isolate_env, fake_id):
+        cfg = build_config()
+        assert cfg.notes_ext == "md"
+
+    def test_taskrc_overrides_defaults(
+        self, fake_rc, monkeypatch, isolate_env, fake_id
+    ):
+        fake_rc.write_text("notes.ext=from-rc\n")
+        cfg = build_config()
+        assert cfg.notes_ext == "from-rc"
+
+    def test_env_overrides_taskrc(self, fake_rc, monkeypatch, isolate_env, fake_id):
+        fake_rc.write_text("notes.ext=from-rc\n")
+        monkeypatch.setenv("TOPEN_NOTES_EXT", "from-env")
+        cfg = build_config()
+        assert cfg.notes_ext == "from-env"
+
+    def test_cli_overrides_env(self, fake_rc, monkeypatch, isolate_env):
+        fake_rc.write_text("notes.ext=from-rc\n")
+        monkeypatch.setenv("TOPEN_NOTES_EXT", "from-env")
+        monkeypatch.setattr("sys.argv", ["topen", "0", "--extension", "from-cli"])
+        cfg = build_config()
+        assert cfg.notes_ext == "from-cli"
+
+    def test_cli_overrides_everything(self, fake_rc, monkeypatch, isolate_env):
+        fake_rc.write_text("notes.ext=from-rc\nnotes.dir=/rc-dir\nnotes.editor=joe")
+        monkeypatch.setenv("TOPEN_NOTES_EXT", "from-env")
+        monkeypatch.setenv("TOPEN_NOTES_DIR", "/env-dir")
+        monkeypatch.setenv("EDITOR", "emacs")
+        # CLI wins
+        monkeypatch.setattr(
+            "sys.argv",
+            [
+                "topen",
+                "0",
+                "--extension",
+                "cli-ext",
+                "--notes-dir",
+                "cli-dir",
+                "--editor",
+                "helix",
+            ],
+        )
+        cfg = build_config()
+        assert cfg.notes_ext == "cli-ext"
+        assert cfg.notes_dir == Path("cli-dir")
+        assert cfg.notes_editor == "helix"
+
+    # sanity check that the task-id coming from CLI is preserved
+    def test_cli_supplies_task_id(self, fake_rc, monkeypatch, isolate_env):
+        monkeypatch.setattr("sys.argv", ["topen", "42"])
+        cfg = build_config()
+        assert cfg.task_id == "42"

test/test_parse_cli.py

@@ -0,0 +1,48 @@
+from pathlib import Path
+
+from topen import parse_cli
+
+
+class TestCli:
+    def test_cli_minimum_id(self, monkeypatch):
+        monkeypatch.setattr("sys.argv", ["topen", "42"])
+        assert parse_cli() == {"task_id": "42"}
+
+    def test_cli_options(self, monkeypatch):
+        monkeypatch.setattr(
+            "sys.argv",
+            [
+                "topen",
+                "123",
+                "--extension",
+                "txt",
+                "--editor",
+                "vim",
+                "--annotation",
+                "HERENOTE",
+            ],
+        )
+        assert parse_cli() == {
+            "task_id": "123",
+            "notes_ext": "txt",
+            "notes_editor": "vim",
+            "notes_annot": "HERENOTE",
+        }
+
+    def test_cli_notes_quiet_is_flag(self, monkeypatch):
+        monkeypatch.setattr(
+            "sys.argv",
+            [
+                "topen",
+                "123",
+                "--quiet",
+            ],
+        )
+        assert parse_cli()["notes_quiet"] is True
+
+    def test_cli_parses_paths(self, monkeypatch):
+        monkeypatch.setattr(
+            "sys.argv",
+            ["topen", "123", "--notes-dir", "/somewhere/else"],
+        )
+        assert parse_cli()["notes_dir"] == Path("/somewhere/else")

test/test_parse_env.py

@@ -0,0 +1,35 @@
+from pathlib import Path
+
+from topen import parse_env
+
+
+class TestEnv:
+    def test_env_notes_ext(self, isolate_env, monkeypatch):
+        monkeypatch.setenv("TOPEN_NOTES_DIR", "/blablubb")
+        monkeypatch.setenv("TOPEN_NOTES_EXT", "rst")
+        monkeypatch.setenv("TOPEN_NOTES_ANNOT", "qmd")
+        monkeypatch.setenv("TOPEN_NOTES_EDITOR", "vim")
+        monkeypatch.setenv("TOPEN_NOTES_QUIET", "true")
+        env = parse_env()
+        assert env["notes_dir"] == Path("/blablubb")
+        assert env["notes_ext"] == "rst"
+        assert env["notes_annot"] == "qmd"
+        assert env["notes_editor"] == "vim"
+        assert env["notes_quiet"] is True
+
+    def test_env_task_rc(self, isolate_env, monkeypatch):
+        monkeypatch.setenv("TASKRC", "/a/dir/that/dont/exist/file")
+        monkeypatch.setenv("TASKDATA", "~/somewhere/tasks")
+        env = parse_env()
+        assert env["task_rc"] == Path("/a/dir/that/dont/exist/file")
+        assert env["task_data"] == Path("~/somewhere/tasks")
+
+    def test_env_parses_boolean_true(self, isolate_env, monkeypatch):
+        monkeypatch.setenv("TOPEN_NOTES_QUIET", "true")
+        env = parse_env()
+        assert env["notes_quiet"] is True
+
+    def test_env_parses_boolean_false(self, isolate_env, monkeypatch):
+        monkeypatch.setenv("TOPEN_NOTES_QUIET", "false")
+        env = parse_env()
+        assert env["notes_quiet"] is False

test/test_parse_rc.py

@@ -0,0 +1,36 @@
+from pathlib import Path
+
+from topen import parse_rc
+
+
+class TestRcFile:
+    def test_taskrc_parsing(self, fake_rc):
+        fake_rc.write_text("""
+        data.location=~/.taskies
+        notes.dir=/there
+        notes.ext=yaml
+        notes.annot=Boo!
+        notes.editor=micro
+        notes.quiet=true
+        """)
+        rc_cfg = parse_rc(fake_rc)
+        assert rc_cfg["task_data"] == Path("~/.taskies")
+        assert rc_cfg["notes_dir"] == Path("/there")
+        assert rc_cfg["notes_ext"] == "yaml"
+        assert rc_cfg["notes_annot"] == "Boo!"
+        assert rc_cfg["notes_editor"] == "micro"
+        assert rc_cfg["notes_quiet"] is True
+
+    def test_taskrc_parses_boolean_true(self, fake_rc):
+        fake_rc.write_text("""
+        notes.quiet=true
+        """)
+        rc_cfg = parse_rc(fake_rc)
+        assert rc_cfg["notes_quiet"] is True
+
+    def test_taskrc_parses_boolean_false(self, fake_rc):
+        fake_rc.write_text("""
+        notes.quiet=false
+        """)
+        rc_cfg = parse_rc(fake_rc)
+        assert rc_cfg["notes_quiet"] is False

topen.py

@@ -63,11 +63,9 @@ def main(cfg: "TConf | None" = None, io: "_IO | None" = None) -> int:
 
     fpath = get_notes_file(uuid, notes_dir=cfg.notes_dir, notes_ext=cfg.notes_ext)
 
-    if not fpath.parent.exists():
+    _ensure_parent_dir(fpath)
-        fpath.parent.mkdir(parents=True, exist_ok=True)
-
     io.out(f"Editing note: {fpath}")
-    open_editor(fpath, editor=cfg.notes_editor)
+    open_editor(fpath, editor=cfg.notes_editor, io=io)
 
     if fpath.exists():
         if is_annotation_missing(task, annotation_content=cfg.notes_annot):
@@ -97,9 +95,13 @@ def get_notes_file(uuid: str, notes_dir: Path, notes_ext: str) -> Path:
     return Path(notes_dir).joinpath(f"{uuid}.{notes_ext}")
 
 
-def open_editor(file: Path, editor: str) -> None:
+def open_editor(file: Path, editor: str, io: "_IO | None" = None) -> None:
     """Opens a file with the chosen editor."""
-    _ = subprocess.run(f"{editor} {file}", shell=True)
+    try:
+        _ = subprocess.run([editor, str(file)], check=True)
+    except subprocess.CalledProcessError:
+        if io:
+            io.err("Editor exited with an error, aborting.\n")
 
 
 def is_annotation_missing(task: Task, annotation_content: str) -> bool:
@@ -132,19 +134,24 @@ class Opt:
     metavar: str | None = None
     cast: type | Callable = str
     help_text: str = ""
-    flag: bool = False
+    is_flag: bool = False
 
 
-def _real_path(p: Path | str) -> Path:
+def _expand_path(p: Path | str) -> Path:
     return Path(os.path.expandvars(p)).expanduser()
 
 
+def _ensure_parent_dir(file: Path) -> None:
+    if not file.parent.exists():
+        file.parent.mkdir(parents=True, exist_ok=True)
+
+
 def _determine_default_task_rc() -> Path:
-    if _real_path("~/.taskrc").exists():
+    if _expand_path("~/.taskrc").exists():
-        return _real_path("~/.taskrc")
+        return _expand_path("~/.taskrc")
-    if _real_path("$XDG_CONFIG_HOME/task/taskrc").exists():
+    if _expand_path("$XDG_CONFIG_HOME/task/taskrc").exists():
-        return _real_path("$XDG_CONFIG_HOME/task/taskrc")
+        return _expand_path("$XDG_CONFIG_HOME/task/taskrc")
-    return _real_path("~/.config/task/taskrc")
+    return _expand_path("~/.config/task/taskrc")
 
 
 def _strtobool(val: str) -> bool:
@@ -224,7 +231,8 @@ OPTIONS: dict[str, Opt] = {
         "notes.quiet",
         default=False,
         cast=_strtobool,
-        help_text="Silence any verbose displayed information",
+        help_text="Silence any verbosely displayed information",
+        is_flag=True,
     ),
 }
 
@@ -257,11 +265,11 @@ class TConf:
     """If set topen will give no feedback on note editing."""
 
     def __post_init__(self):
-        self.task_rc = _real_path(self.task_rc)
+        self.task_rc = _expand_path(self.task_rc)
-        self.task_data = _real_path(self.task_data)
+        self.task_data = _expand_path(self.task_data)
         if self.notes_dir == NON_EXISTENT_PATH:
             self.notes_dir = self._default_notes_dir()
-        self.notes_dir = _real_path(self.notes_dir)
+        self.notes_dir = _expand_path(self.notes_dir)
         if not self.notes_editor:
             self.notes_editor = (
                 os.getenv("EDITOR")
@@ -321,6 +329,15 @@ you view the task.
     for key, opt in OPTIONS.items():
         if opt.cli is None:
             continue
+        if opt.is_flag:
+            parser.add_argument(
+                *opt.cli,
+                dest=key,
+                help=opt.help_text,
+                default=None,
+                action="store_true",
+            )
+            continue
         parser.add_argument(
             *opt.cli,
             dest=key,