Skip to content

Commit 00cb3a2

Browse files
committed
fix(config[symlink-format]) prefer target suffix for symlinked configs
why: The first symlink-path fix preserved visible config names, which fixed extensionless dotfile targets but broke alias paths and mismatched symlinks. An explicit alias like `vcspull-config -> .vcspull.yaml` was rejected for having no suffix, and a home link like `.vcspull.yaml -> vcspull.json` could be rewritten with YAML because later format checks only saw the visible path. what: - Add shared config-format detection that prefers a symlink target's supported suffix and otherwise falls back to the visible path suffix - Use that helper for config reads, duplicate-aware YAML loading, save dispatch, ordered-item saves, and import config-path validation - Add regressions for target-format precedence, extensionless alias paths, and end-to-end JSON writes through a symlinked home config
1 parent 927c308 commit 00cb3a2

7 files changed

Lines changed: 130 additions & 24 deletions

File tree

src/vcspull/_internal/config_reader.py

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,56 @@
88
import yaml
99

1010
FormatLiteral = t.Literal["json", "yaml"]
11+
_SUPPORTED_CONFIG_SUFFIXES: dict[str, FormatLiteral] = {
12+
".json": "json",
13+
".yaml": "yaml",
14+
".yml": "yaml",
15+
}
16+
17+
18+
def config_format_from_path(path: pathlib.Path) -> FormatLiteral | None:
19+
"""Return config format inferred from a path or symlink target.
20+
21+
The visible path remains the config identity, but symlinks may point
22+
at a differently named target. When the target advertises a supported
23+
config suffix, prefer that format; otherwise fall back to the visible
24+
path suffix.
25+
26+
Parameters
27+
----------
28+
path : pathlib.Path
29+
Path to inspect.
30+
31+
Returns
32+
-------
33+
FormatLiteral | None
34+
``"json"`` or ``"yaml"`` when a supported suffix is found,
35+
otherwise ``None``.
36+
37+
Examples
38+
--------
39+
>>> config_format_from_path(pathlib.Path("config.yaml"))
40+
'yaml'
41+
>>> import tempfile
42+
>>> with tempfile.TemporaryDirectory() as tmp:
43+
... root = pathlib.Path(tmp)
44+
... target = root / "config.json"
45+
... _ = target.write_text("{}", encoding="utf-8")
46+
... link = root / ".vcspull.yaml"
47+
... link.symlink_to(target)
48+
... config_format_from_path(link)
49+
'json'
50+
"""
51+
path_format = _SUPPORTED_CONFIG_SUFFIXES.get(path.suffix.lower())
52+
53+
target_format: FormatLiteral | None = None
54+
if path.is_symlink():
55+
resolved = path.resolve(strict=False)
56+
target_format = _SUPPORTED_CONFIG_SUFFIXES.get(resolved.suffix.lower())
57+
58+
return target_format or path_format
59+
60+
1161
RawConfigData: t.TypeAlias = dict[t.Any, t.Any]
1262

1363

@@ -117,11 +167,8 @@ def _from_file(cls, path: pathlib.Path) -> dict[str, t.Any]:
117167
# Revisit once the new ``vcspull add`` flow lands so both commands share
118168
# the same duplication safeguards.
119169

120-
if path.suffix in {".yaml", ".yml"}:
121-
fmt: FormatLiteral = "yaml"
122-
elif path.suffix == ".json":
123-
fmt = "json"
124-
else:
170+
fmt = config_format_from_path(path)
171+
if fmt is None:
125172
msg = f"{path.suffix} not supported in {path}"
126173
raise NotImplementedError(msg)
127174

@@ -335,7 +382,7 @@ def _load_from_path(
335382
cls,
336383
path: pathlib.Path,
337384
) -> tuple[dict[str, t.Any], dict[str, list[t.Any]], list[tuple[str, t.Any]]]:
338-
if path.suffix.lower() in {".yaml", ".yml"}:
385+
if config_format_from_path(path) == "yaml":
339386
content = path.read_text(encoding="utf-8")
340387
return cls._load_yaml_with_duplicates(content)
341388

src/vcspull/cli/add.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@
1313

1414
from colorama import Fore, Style
1515

16-
from vcspull._internal.config_reader import DuplicateAwareConfigReader
16+
from vcspull._internal.config_reader import (
17+
DuplicateAwareConfigReader,
18+
config_format_from_path,
19+
)
1720
from vcspull._internal.private_path import PrivatePath
1821
from vcspull.config import (
1922
canonicalize_workspace_path,
@@ -341,7 +344,7 @@ def _save_ordered_items(
341344
>>> "~/code/" in data
342345
True
343346
"""
344-
if config_file_path.suffix.lower() == ".json":
347+
if config_format_from_path(config_file_path) == "json":
345348
save_config_json(
346349
config_file_path,
347350
_collapse_ordered_items_to_dict(ordered_items),

src/vcspull/cli/import_cmd/_common.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414
import sys
1515
import typing as t
1616

17-
from vcspull._internal.config_reader import DuplicateAwareConfigReader
17+
from vcspull._internal.config_reader import (
18+
DuplicateAwareConfigReader,
19+
config_format_from_path,
20+
)
1821
from vcspull._internal.private_path import PrivatePath
1922
from vcspull._internal.remotes import (
2023
AuthenticationError,
@@ -565,7 +568,7 @@ def _resolve_config_file(config_path_str: str | None) -> pathlib.Path:
565568
"""
566569
if config_path_str:
567570
path = normalize_config_file_path(pathlib.Path(config_path_str))
568-
if path.suffix.lower() not in {".yaml", ".yml", ".json"}:
571+
if config_format_from_path(path) is None:
569572
msg = f"Unsupported config file type: {path.suffix}"
570573
raise ValueError(msg)
571574
return path

src/vcspull/config.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@
1818
from vcspull.validator import is_valid_config
1919

2020
from . import exc
21-
from ._internal.config_reader import ConfigReader, DuplicateAwareConfigReader
21+
from ._internal.config_reader import (
22+
ConfigReader,
23+
DuplicateAwareConfigReader,
24+
config_format_from_path,
25+
)
2226
from .types import ConfigDict, RawConfigDict, WorktreeConfigDict
2327
from .util import get_config_dir, update_dict
2428

@@ -849,7 +853,7 @@ def save_config(config_file_path: pathlib.Path, data: dict[t.Any, t.Any]) -> Non
849853
... "repo" in p.read_text()
850854
True
851855
"""
852-
if config_file_path.suffix.lower() == ".json":
856+
if config_format_from_path(config_file_path) == "json":
853857
save_config_json(config_file_path, data)
854858
else:
855859
save_config_yaml(config_file_path, data)

tests/cli/test_add.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from __future__ import annotations
44

55
import argparse
6+
import json
67
import logging
78
import os
89
import pathlib
@@ -367,6 +368,44 @@ def test_add_repo_uses_home_symlink_config_without_losing_yaml_suffix(
367368
assert "git+https://github.com/user/newrepo.git" in config_text
368369

369370

371+
def test_add_repo_uses_target_json_format_for_home_symlink(
372+
tmp_path: pathlib.Path,
373+
monkeypatch: MonkeyPatch,
374+
) -> None:
375+
"""Default home symlinks should save using the supported target format."""
376+
monkeypatch.setenv("HOME", str(tmp_path))
377+
monkeypatch.chdir(tmp_path)
378+
379+
dotfiles_dir = tmp_path / "dotfiles"
380+
dotfiles_dir.mkdir()
381+
382+
real_config = dotfiles_dir / "vcspull.json"
383+
real_config.write_text("{}\n", encoding="utf-8")
384+
385+
symlink = tmp_path / ".vcspull.yaml"
386+
symlink.symlink_to(real_config)
387+
388+
repo_path = tmp_path / "code" / "jsonrepo"
389+
repo_path.mkdir(parents=True)
390+
391+
add_repo(
392+
name="jsonrepo",
393+
url="git+https://github.com/user/jsonrepo.git",
394+
config_file_path_str=None,
395+
path=str(repo_path),
396+
workspace_root_path="~/code/",
397+
dry_run=False,
398+
)
399+
400+
assert symlink.is_symlink()
401+
assert symlink.resolve() == real_config.resolve()
402+
403+
data = json.loads(real_config.read_text(encoding="utf-8"))
404+
assert data["~/code/"]["jsonrepo"]["repo"] == (
405+
"git+https://github.com/user/jsonrepo.git"
406+
)
407+
408+
370409
def test_add_repo_invalid_config_logs_private_path(
371410
user_path: pathlib.Path,
372411
caplog: pytest.LogCaptureFixture,

tests/cli/test_import_repos.py

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -188,26 +188,23 @@ def test_resolve_config_file(
188188
assert result.name == expected_suffix
189189

190190

191-
def test_resolve_config_file_preserves_symlink_suffix(
191+
def test_resolve_config_file_accepts_extensionless_symlink_alias(
192192
tmp_path: pathlib.Path,
193193
monkeypatch: MonkeyPatch,
194194
) -> None:
195-
"""Explicit config paths should keep symlink suffixes for format detection."""
195+
"""Explicit alias paths should inherit a supported target config format."""
196196
monkeypatch.setenv("HOME", str(tmp_path))
197197

198-
dotfiles_dir = tmp_path / "dotfiles"
199-
dotfiles_dir.mkdir()
200-
201-
real_config = dotfiles_dir / "vcspull-config"
198+
real_config = tmp_path / ".vcspull.yaml"
202199
real_config.write_text("~/repos/: {}\n", encoding="utf-8")
203200

204-
symlink = tmp_path / ".vcspull.yaml"
205-
symlink.symlink_to(real_config)
201+
alias_path = tmp_path / "vcspull-config"
202+
alias_path.symlink_to(real_config)
206203

207-
result = _resolve_config_file(str(symlink))
204+
result = _resolve_config_file(str(alias_path))
208205

209-
assert result == symlink
210-
assert result.suffix == ".yaml"
206+
assert result == alias_path
207+
assert result.suffix == ""
211208
assert result.resolve() == real_config.resolve()
212209

213210

tests/test_config_file.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import pytest
1010

1111
from vcspull import config, exc
12-
from vcspull._internal.config_reader import ConfigReader
12+
from vcspull._internal.config_reader import ConfigReader, config_format_from_path
1313
from vcspull.config import expand_dir, extract_repos
1414
from vcspull.validator import is_valid_config
1515

@@ -267,6 +267,19 @@ def test_find_home_config_files_preserves_symlink_suffix(
267267
assert results[0].resolve() == real_file.resolve()
268268

269269

270+
def test_config_format_from_path_prefers_supported_symlink_target(
271+
tmp_path: pathlib.Path,
272+
) -> None:
273+
"""Symlink targets with supported suffixes should determine the config format."""
274+
real_json = tmp_path / "vcspull.json"
275+
real_json.write_text("{}\n", encoding="utf-8")
276+
277+
mismatch = tmp_path / ".vcspull.yaml"
278+
mismatch.symlink_to(real_json)
279+
280+
assert config_format_from_path(mismatch) == "json"
281+
282+
270283
def test_in_dir(
271284
config_path: pathlib.Path,
272285
yaml_config: pathlib.Path,

0 commit comments

Comments
 (0)