From 953eb6dcc1d84cc3ec675ad578d4d42ed093acdc Mon Sep 17 00:00:00 2001 From: David Hong Date: Wed, 5 Aug 2026 14:57:03 -0700 Subject: [PATCH 1/2] fix: preserve inter-document separators regardless of explicit_start setting - Change explicit_start type from bool to Optional[bool] - Default to None (leave document markers as-is) - Never remove inter-document --- separators (structurally required) - explicit_start=False now only affects the first document marker Fixes #274 Fixes #307 --- src/yamlfix/adapters.py | 31 +++++- src/yamlfix/model.py | 2 +- tests/unit/test_adapter_yaml.py | 16 +++ tests/unit/test_explicit_start.py | 165 ++++++++++++++++++++++++++++++ tests/unit/test_services.py | 8 +- 5 files changed, 215 insertions(+), 7 deletions(-) create mode 100644 tests/unit/test_explicit_start.py diff --git a/src/yamlfix/adapters.py b/src/yamlfix/adapters.py index dbb845b..f91e934 100644 --- a/src/yamlfix/adapters.py +++ b/src/yamlfix/adapters.py @@ -58,7 +58,8 @@ def _base_configuration(self) -> None: # Start the document with --- # ignore: variable has type None, what can we do, it doesn't have type hints... - self.yaml.explicit_start = config.explicit_start # type: ignore + if config.explicit_start is not None: + self.yaml.explicit_start = config.explicit_start # type: ignore self.yaml.width = config.line_length # type: ignore self.yaml.preserve_quotes = config.preserve_quotes # type: ignore @@ -374,15 +375,37 @@ def _ruamel_yaml_fixer(self, source_code: str) -> str: Corrected source code. """ log.debug("Running ruamel yaml fixer...") - source_dicts = self.yaml.load_all(source_code) + + # Detect if the original source starts with an explicit document marker + original_has_explicit_start = source_code.lstrip().startswith("---") + + source_dicts = list(self.yaml.load_all(source_code)) + + # When explicit_start is None (leave as-is), temporarily set ruyaml's + # explicit_start based on whether the original source had it + restore_explicit_start = False + if self.config.explicit_start is None: + restore_explicit_start = True + self.yaml.explicit_start = original_has_explicit_start # type: ignore + + # Determine if ruyaml will emit --- for each document + yaml_emits_start = bool(self.yaml.explicit_start) # Return the output to a string string_stream = StringIO() - for source_dict in source_dicts: + for i, source_dict in enumerate(source_dicts): + if i > 0 and not yaml_emits_start: + # Inter-document separators are structurally required; + # only add manually if ruyaml isn't already emitting them + string_stream.write("---\n") self.yaml.dump(source_dict, string_stream) - source_code = string_stream.getvalue() + source_code = string_stream.getvalue() string_stream.close() + # Restore ruyaml's explicit_start to None if we changed it + if restore_explicit_start: + self.yaml.explicit_start = None # type: ignore + return source_code.strip() @staticmethod diff --git a/src/yamlfix/model.py b/src/yamlfix/model.py index 780e883..44a5e87 100644 --- a/src/yamlfix/model.py +++ b/src/yamlfix/model.py @@ -24,7 +24,7 @@ class YamlfixConfig(BaseModel): whitelines: int = 0 section_whitelines: int = 0 config_path: Optional[str] = None - explicit_start: bool = True + explicit_start: Optional[bool] = None indent_mapping: int = 2 indent_offset: int = 2 indent_sequence: int = 4 diff --git a/tests/unit/test_adapter_yaml.py b/tests/unit/test_adapter_yaml.py index b56739d..10e86df 100644 --- a/tests/unit/test_adapter_yaml.py +++ b/tests/unit/test_adapter_yaml.py @@ -47,6 +47,7 @@ def test_indentation_config(self) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.indent_offset = 4 config.indent_mapping = 4 config.indent_sequence = 8 @@ -141,6 +142,7 @@ def test_if_line_length_expands(self) -> None: """ # noqa: E501 ) config = YamlfixConfig() + config.explicit_start = True config.line_length = 100 result = fix_code(source, config) @@ -175,6 +177,7 @@ def test_if_line_length_contracts(self) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.line_length = 20 result = fix_code(source, config) @@ -207,6 +210,7 @@ def test_none_representation_config(self, none_representation: str) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.none_representation = none_representation result = fix_code(source, config) @@ -278,6 +282,7 @@ def test_quote_all_keys_and_values_config(self, quote_representation: str) -> No """ ) config = YamlfixConfig() + config.explicit_start = True config.quote_representation = quote_representation config.quote_keys_and_basic_values = True @@ -333,6 +338,7 @@ def test_quote_values_config(self, quote_representation: str) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.quote_representation = quote_representation config.quote_basic_values = True @@ -391,6 +397,7 @@ def test_quote_all_keys_and_values_config_and_preserve_quotes( """ ) config = YamlfixConfig() + config.explicit_start = True config.quote_representation = quote_representation config.quote_keys_and_basic_values = True config.preserve_quotes = True @@ -450,6 +457,7 @@ def test_quote_values_config_and_preserve_quotes( """ ) config = YamlfixConfig() + config.explicit_start = True config.quote_representation = quote_representation config.quote_basic_values = True config.preserve_quotes = True @@ -476,6 +484,7 @@ def test_sequence_flow_style_config(self) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.sequence_style = YamlNodeStyle.FLOW_STYLE result = fix_code(source, config) @@ -504,6 +513,7 @@ def test_sequence_block_style_config(self) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.sequence_style = YamlNodeStyle.BLOCK_STYLE result = fix_code(source, config) @@ -566,6 +576,7 @@ def test_sequence_block_style_enforcement_for_lists_with_comments(self) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.sequence_style = YamlNodeStyle.FLOW_STYLE result = fix_code(source, config) @@ -606,6 +617,7 @@ def test_sequence_block_style_enforcement_for_lists_with_non_scalar_values( """ ) config = YamlfixConfig() + config.explicit_start = True config.sequence_style = YamlNodeStyle.FLOW_STYLE result = fix_code(source, config) @@ -650,6 +662,7 @@ def test_sequence_block_style_enforcement_for_lists_longer_than_line_length( """ ) config = YamlfixConfig() + config.explicit_start = True config.line_length = 40 config.sequence_style = YamlNodeStyle.FLOW_STYLE @@ -709,6 +722,7 @@ def test_sequence_flow_style_with_trailing_newlines(self) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.sequence_style = YamlNodeStyle.FLOW_STYLE result = fix_code(source, config) @@ -861,6 +875,7 @@ def test_whitelines_collapsed(self) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True result = fix_code(source, config) @@ -899,6 +914,7 @@ def test_whitelines_adjusted_to_value(self) -> None: """ ) config = YamlfixConfig() + config.explicit_start = True config.whitelines = 1 result = fix_code(source, config) diff --git a/tests/unit/test_explicit_start.py b/tests/unit/test_explicit_start.py new file mode 100644 index 0000000..1c12866 --- /dev/null +++ b/tests/unit/test_explicit_start.py @@ -0,0 +1,165 @@ +"""Test explicit_start configuration behavior.""" + +from textwrap import dedent + +from yamlfix.model import YamlfixConfig +from yamlfix.services import fix_code + + +class TestExplicitStartNone: + """Tests for explicit_start = None (leave as-is).""" + + def test_preserves_existing_start_marker(self) -> None: + """When explicit_start is None, existing --- at start is preserved.""" + source = dedent( + """\ + --- + project_name: yamlfix + """ + ) + config = YamlfixConfig() + config.explicit_start = None + + result = fix_code(source, config) + + assert result == source + + def test_does_not_add_start_marker(self) -> None: + """When explicit_start is None, --- is not added to files without it.""" + source = dedent( + """\ + project_name: yamlfix + """ + ) + config = YamlfixConfig() + config.explicit_start = None + + result = fix_code(source, config) + + assert result == source + + +class TestExplicitStartFalse: + """Tests for explicit_start = False.""" + + def test_removes_leading_start_marker(self) -> None: + """When explicit_start is False, leading --- is removed.""" + source = dedent( + """\ + --- + project_name: yamlfix + """ + ) + fixed_source = dedent( + """\ + project_name: yamlfix + """ + ) + config = YamlfixConfig() + config.explicit_start = False + + result = fix_code(source, config) + + assert result == fixed_source + + def test_preserves_inter_document_separators(self) -> None: + """When explicit_start is False, inter-document --- separators are preserved.""" + source = dedent( + """\ + --- + name: doc1 + --- + name: doc2 + """ + ) + config = YamlfixConfig() + config.explicit_start = False + + result = fix_code(source, config) + + # The inter-document separator must remain (structurally required) + assert "---\nname: doc2" in result + + +class TestExplicitStartTrue: + """Tests for explicit_start = True.""" + + def test_adds_start_marker(self) -> None: + """When explicit_start is True, --- is added.""" + source = dedent( + """\ + project_name: yamlfix + """ + ) + fixed_source = dedent( + """\ + --- + project_name: yamlfix + """ + ) + config = YamlfixConfig() + config.explicit_start = True + + result = fix_code(source, config) + + assert result == fixed_source + + +class TestMultiDocumentSeparators: + """Tests for multi-document YAML files.""" + + def test_multi_document_preserves_separators_with_none(self) -> None: + """Multi-document files always keep --- dividers with explicit_start=None.""" + source = dedent( + """\ + --- + name: doc1 + --- + name: doc2 + --- + name: doc3 + """ + ) + config = YamlfixConfig() + config.explicit_start = None + + result = fix_code(source, config) + + # All inter-document separators must be present + assert result.count("---") >= 2 # At least 2 separators between 3 docs + + def test_multi_document_preserves_separators_with_true(self) -> None: + """Multi-document files always keep --- dividers with explicit_start=True.""" + source = dedent( + """\ + --- + name: doc1 + --- + name: doc2 + """ + ) + config = YamlfixConfig() + config.explicit_start = True + + result = fix_code(source, config) + + # Both documents should have separators + assert result.count("---") >= 2 + + def test_multi_document_preserves_separators_with_false(self) -> None: + """Multi-document files always keep --- dividers with explicit_start=False.""" + source = dedent( + """\ + --- + name: doc1 + --- + name: doc2 + """ + ) + config = YamlfixConfig() + config.explicit_start = False + + result = fix_code(source, config) + + # Inter-document separator must remain even with explicit_start=False + assert "---" in result diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index ca23f1b..7736b0f 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -53,8 +53,10 @@ def test_fix_files_can_process_string_arguments(self, tmp_path: Path) -> None: program: yamlfix """ ) + config = YamlfixConfig() + config.explicit_start = True - fix_files([str(test_file)], False) # act + fix_files([str(test_file)], False, config) # act assert test_file.read_text() == fixed_source @@ -124,8 +126,10 @@ def test_fix_code_adds_header(self) -> None: program: yamlfix """ ) + config = YamlfixConfig() + config.explicit_start = True - result = fix_code(source) + result = fix_code(source, config) assert result == fixed_source From b26e7b57a20b929992154892192b3b972bc20940 Mon Sep 17 00:00:00 2001 From: David Hong Date: Wed, 5 Aug 2026 15:25:26 -0700 Subject: [PATCH 2/2] fix: simplify to bool-only, always preserve inter-document separators - Keep explicit_start as bool (default: True), no Optional/None state - explicit_start=False removes leading --- but ALWAYS preserves inter-document --- separators (structurally required for valid YAML) - Remove unused type: ignore comment (fixes mypy CI) - Revert test changes that were for the Optional[bool] approach Fixes #274 Fixes #307 --- src/yamlfix/adapters.py | 21 ++----- src/yamlfix/model.py | 2 +- tests/unit/test_adapter_yaml.py | 16 ----- tests/unit/test_explicit_start.py | 99 +++++++------------------------ tests/unit/test_services.py | 8 +-- 5 files changed, 29 insertions(+), 117 deletions(-) diff --git a/src/yamlfix/adapters.py b/src/yamlfix/adapters.py index f91e934..95b2789 100644 --- a/src/yamlfix/adapters.py +++ b/src/yamlfix/adapters.py @@ -58,8 +58,7 @@ def _base_configuration(self) -> None: # Start the document with --- # ignore: variable has type None, what can we do, it doesn't have type hints... - if config.explicit_start is not None: - self.yaml.explicit_start = config.explicit_start # type: ignore + self.yaml.explicit_start = config.explicit_start # type: ignore self.yaml.width = config.line_length # type: ignore self.yaml.preserve_quotes = config.preserve_quotes # type: ignore @@ -377,17 +376,8 @@ def _ruamel_yaml_fixer(self, source_code: str) -> str: log.debug("Running ruamel yaml fixer...") # Detect if the original source starts with an explicit document marker - original_has_explicit_start = source_code.lstrip().startswith("---") - source_dicts = list(self.yaml.load_all(source_code)) - # When explicit_start is None (leave as-is), temporarily set ruyaml's - # explicit_start based on whether the original source had it - restore_explicit_start = False - if self.config.explicit_start is None: - restore_explicit_start = True - self.yaml.explicit_start = original_has_explicit_start # type: ignore - # Determine if ruyaml will emit --- for each document yaml_emits_start = bool(self.yaml.explicit_start) @@ -395,17 +385,14 @@ def _ruamel_yaml_fixer(self, source_code: str) -> str: string_stream = StringIO() for i, source_dict in enumerate(source_dicts): if i > 0 and not yaml_emits_start: - # Inter-document separators are structurally required; - # only add manually if ruyaml isn't already emitting them + # Inter-document separators are structurally required for + # multi-document YAML; always preserve them even when + # explicit_start is False (which only controls the leading ---) string_stream.write("---\n") self.yaml.dump(source_dict, string_stream) source_code = string_stream.getvalue() string_stream.close() - # Restore ruyaml's explicit_start to None if we changed it - if restore_explicit_start: - self.yaml.explicit_start = None # type: ignore - return source_code.strip() @staticmethod diff --git a/src/yamlfix/model.py b/src/yamlfix/model.py index 44a5e87..780e883 100644 --- a/src/yamlfix/model.py +++ b/src/yamlfix/model.py @@ -24,7 +24,7 @@ class YamlfixConfig(BaseModel): whitelines: int = 0 section_whitelines: int = 0 config_path: Optional[str] = None - explicit_start: Optional[bool] = None + explicit_start: bool = True indent_mapping: int = 2 indent_offset: int = 2 indent_sequence: int = 4 diff --git a/tests/unit/test_adapter_yaml.py b/tests/unit/test_adapter_yaml.py index 10e86df..b56739d 100644 --- a/tests/unit/test_adapter_yaml.py +++ b/tests/unit/test_adapter_yaml.py @@ -47,7 +47,6 @@ def test_indentation_config(self) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.indent_offset = 4 config.indent_mapping = 4 config.indent_sequence = 8 @@ -142,7 +141,6 @@ def test_if_line_length_expands(self) -> None: """ # noqa: E501 ) config = YamlfixConfig() - config.explicit_start = True config.line_length = 100 result = fix_code(source, config) @@ -177,7 +175,6 @@ def test_if_line_length_contracts(self) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.line_length = 20 result = fix_code(source, config) @@ -210,7 +207,6 @@ def test_none_representation_config(self, none_representation: str) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.none_representation = none_representation result = fix_code(source, config) @@ -282,7 +278,6 @@ def test_quote_all_keys_and_values_config(self, quote_representation: str) -> No """ ) config = YamlfixConfig() - config.explicit_start = True config.quote_representation = quote_representation config.quote_keys_and_basic_values = True @@ -338,7 +333,6 @@ def test_quote_values_config(self, quote_representation: str) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.quote_representation = quote_representation config.quote_basic_values = True @@ -397,7 +391,6 @@ def test_quote_all_keys_and_values_config_and_preserve_quotes( """ ) config = YamlfixConfig() - config.explicit_start = True config.quote_representation = quote_representation config.quote_keys_and_basic_values = True config.preserve_quotes = True @@ -457,7 +450,6 @@ def test_quote_values_config_and_preserve_quotes( """ ) config = YamlfixConfig() - config.explicit_start = True config.quote_representation = quote_representation config.quote_basic_values = True config.preserve_quotes = True @@ -484,7 +476,6 @@ def test_sequence_flow_style_config(self) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.sequence_style = YamlNodeStyle.FLOW_STYLE result = fix_code(source, config) @@ -513,7 +504,6 @@ def test_sequence_block_style_config(self) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.sequence_style = YamlNodeStyle.BLOCK_STYLE result = fix_code(source, config) @@ -576,7 +566,6 @@ def test_sequence_block_style_enforcement_for_lists_with_comments(self) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.sequence_style = YamlNodeStyle.FLOW_STYLE result = fix_code(source, config) @@ -617,7 +606,6 @@ def test_sequence_block_style_enforcement_for_lists_with_non_scalar_values( """ ) config = YamlfixConfig() - config.explicit_start = True config.sequence_style = YamlNodeStyle.FLOW_STYLE result = fix_code(source, config) @@ -662,7 +650,6 @@ def test_sequence_block_style_enforcement_for_lists_longer_than_line_length( """ ) config = YamlfixConfig() - config.explicit_start = True config.line_length = 40 config.sequence_style = YamlNodeStyle.FLOW_STYLE @@ -722,7 +709,6 @@ def test_sequence_flow_style_with_trailing_newlines(self) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.sequence_style = YamlNodeStyle.FLOW_STYLE result = fix_code(source, config) @@ -875,7 +861,6 @@ def test_whitelines_collapsed(self) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True result = fix_code(source, config) @@ -914,7 +899,6 @@ def test_whitelines_adjusted_to_value(self) -> None: """ ) config = YamlfixConfig() - config.explicit_start = True config.whitelines = 1 result = fix_code(source, config) diff --git a/tests/unit/test_explicit_start.py b/tests/unit/test_explicit_start.py index 1c12866..9c14f98 100644 --- a/tests/unit/test_explicit_start.py +++ b/tests/unit/test_explicit_start.py @@ -6,39 +6,6 @@ from yamlfix.services import fix_code -class TestExplicitStartNone: - """Tests for explicit_start = None (leave as-is).""" - - def test_preserves_existing_start_marker(self) -> None: - """When explicit_start is None, existing --- at start is preserved.""" - source = dedent( - """\ - --- - project_name: yamlfix - """ - ) - config = YamlfixConfig() - config.explicit_start = None - - result = fix_code(source, config) - - assert result == source - - def test_does_not_add_start_marker(self) -> None: - """When explicit_start is None, --- is not added to files without it.""" - source = dedent( - """\ - project_name: yamlfix - """ - ) - config = YamlfixConfig() - config.explicit_start = None - - result = fix_code(source, config) - - assert result == source - - class TestExplicitStartFalse: """Tests for explicit_start = False.""" @@ -80,6 +47,26 @@ def test_preserves_inter_document_separators(self) -> None: # The inter-document separator must remain (structurally required) assert "---\nname: doc2" in result + def test_multi_document_three_docs(self) -> None: + """When explicit_start is False, all inter-document separators are preserved.""" + source = dedent( + """\ + --- + name: doc1 + --- + name: doc2 + --- + name: doc3 + """ + ) + config = YamlfixConfig() + config.explicit_start = False + + result = fix_code(source, config) + + # Inter-document separators must remain (at least 2 for 3 docs) + assert result.count("---") >= 2 + class TestExplicitStartTrue: """Tests for explicit_start = True.""" @@ -104,32 +91,8 @@ def test_adds_start_marker(self) -> None: assert result == fixed_source - -class TestMultiDocumentSeparators: - """Tests for multi-document YAML files.""" - - def test_multi_document_preserves_separators_with_none(self) -> None: - """Multi-document files always keep --- dividers with explicit_start=None.""" - source = dedent( - """\ - --- - name: doc1 - --- - name: doc2 - --- - name: doc3 - """ - ) - config = YamlfixConfig() - config.explicit_start = None - - result = fix_code(source, config) - - # All inter-document separators must be present - assert result.count("---") >= 2 # At least 2 separators between 3 docs - - def test_multi_document_preserves_separators_with_true(self) -> None: - """Multi-document files always keep --- dividers with explicit_start=True.""" + def test_multi_document_preserves_separators(self) -> None: + """Multi-document files keep --- dividers with explicit_start=True.""" source = dedent( """\ --- @@ -145,21 +108,3 @@ def test_multi_document_preserves_separators_with_true(self) -> None: # Both documents should have separators assert result.count("---") >= 2 - - def test_multi_document_preserves_separators_with_false(self) -> None: - """Multi-document files always keep --- dividers with explicit_start=False.""" - source = dedent( - """\ - --- - name: doc1 - --- - name: doc2 - """ - ) - config = YamlfixConfig() - config.explicit_start = False - - result = fix_code(source, config) - - # Inter-document separator must remain even with explicit_start=False - assert "---" in result diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 7736b0f..ca23f1b 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -53,10 +53,8 @@ def test_fix_files_can_process_string_arguments(self, tmp_path: Path) -> None: program: yamlfix """ ) - config = YamlfixConfig() - config.explicit_start = True - fix_files([str(test_file)], False, config) # act + fix_files([str(test_file)], False) # act assert test_file.read_text() == fixed_source @@ -126,10 +124,8 @@ def test_fix_code_adds_header(self) -> None: program: yamlfix """ ) - config = YamlfixConfig() - config.explicit_start = True - result = fix_code(source, config) + result = fix_code(source) assert result == fixed_source