Skip to content

Commit

Permalink
Don't strip the input of a DictOption file config (pantsbuild#17705)
Browse files Browse the repository at this point in the history
YAML, in particular, is trailing-newline aware, as evidenced by the new test (fails before change). Let's just let the underlying  libraries handle leading/trailing space 😉
  • Loading branch information
thejcannon authored Dec 3, 2022
1 parent 1f0370a commit 11bca3f
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
14 changes: 14 additions & 0 deletions src/python/pants/option/options_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,20 @@ def test_fromfile_yaml(self) -> None:
options = self._parse(flags=f"fromfile --{'dictvalue'}=@{fp.name}")
assert val == options.for_scope("fromfile")["dictvalue"]

def test_fromfile_yaml_trailing_newlines_matter(self) -> None:
with temporary_file(suffix=".yaml", binary_mode=False) as fp:
fp.write(
dedent(
"""\
a: |+
multiline
"""
)
)
fp.close()
options = self._parse(flags=f"fromfile --{'dictvalue'}=@{fp.name}")
assert {"a": "multiline\n"} == options.for_scope("fromfile")["dictvalue"]

def test_fromfile_error(self) -> None:
options = self._parse(flags="fromfile --string=@/does/not/exist")
with pytest.raises(FromfileError):
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/option/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,13 +565,13 @@ def expand(val_or_str):
fromfile = val_or_str[1:]
try:
with open(fromfile) as fp:
s = fp.read().strip()
s = fp.read()
if fromfile.endswith(".json"):
return json.loads(s)
elif fromfile.endswith(".yml") or fromfile.endswith(".yaml"):
return yaml.safe_load(s)
else:
return s
return s.strip()
except (OSError, ValueError, yaml.YAMLError) as e:
raise FromfileError(
f"Failed to read {dest} in {self._scope_str()} from file {fromfile}: {e!r}"
Expand Down

0 comments on commit 11bca3f

Please sign in to comment.