Skip to content

Commit 74d7e37

Browse files
BernBern
Bern
authored and
Bern
committed
fix: Improve handling of multiline values in config merging
- Added proper handling of multiline values with continuation characters - Added support for embedded comments in multiline values - Refactored ConfigMerger.write() method for better multiline detection - Added comprehensive test cases for complex multiline configurations
1 parent 147f6b9 commit 74d7e37

2 files changed

Lines changed: 347 additions & 167 deletions

File tree

‎src/bydefault/utils/merge_utils.py‎

Lines changed: 174 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"""
66

77
import os
8+
import re
89
import shutil
910
from pathlib import Path
1011
from typing import Dict, List, Optional
@@ -15,7 +16,7 @@
1516
MergeResult,
1617
StanzaMergeResult,
1718
)
18-
from bydefault.models.sort_models import Setting, Stanza
19+
from bydefault.models.sort_models import Stanza
1920
from bydefault.utils.change_detection import _parse_conf_file
2021
from bydefault.utils.sort_utils import ConfigSorter
2122

@@ -93,11 +94,6 @@ def write(self) -> None:
9394
9495
This method handles writing merged configurations back to the file system.
9596
It preserves stanza order and comments from the original files.
96-
97-
Note:
98-
Future refactoring: This file writing functionality should be
99-
extracted to a common utility function shared with SortedConfigWriter
100-
to ensure consistent behavior between the merge and sort commands.
10197
"""
10298
for file_result in self.result.file_results:
10399
if not file_result.success:
@@ -106,8 +102,7 @@ def write(self) -> None:
106102
# Get the local file path
107103
local_file = file_result.file_path
108104

109-
# Handle metadata files (they're in metadata directory
110-
# instead of local/default)
105+
# Handle metadata files (they're in metadata directory instead of local/default)
111106
if local_file.name == "local.meta" and "metadata" in str(local_file):
112107
default_file = self.ta_dir / "metadata" / "default.meta"
113108
else:
@@ -119,173 +114,185 @@ def write(self) -> None:
119114
self._copy_file(local_file, default_file)
120115
continue
121116

122-
# Create file content from scratch
123-
local_sorter = ConfigSorter(local_file, verbose=self.verbose)
124-
default_sorter = ConfigSorter(default_file, verbose=self.verbose)
117+
# Parse both files to get complete stanza information
118+
local_parsed = _parse_conf_file(local_file)
119+
default_parsed = _parse_conf_file(default_file)
125120

126-
local_sorter.parse()
127-
default_sorter.parse()
121+
# Read the full local file content for multiline detection
122+
with open(local_file, "r", encoding="utf-8") as f:
123+
local_content = f.read()
128124

129-
# For replace mode, completely replace stanzas from local
130-
if self.mode == MergeMode.REPLACE:
131-
for stanza_name, local_stanza in local_sorter.stanzas.items():
132-
if stanza_name in default_sorter.stanzas:
133-
# Replace all settings in the stanza
134-
default_stanza = default_sorter.stanzas[stanza_name]
135-
default_stanza.settings = {} # Clear existing settings
136-
137-
# Copy settings from local to default
138-
for setting_key, setting in local_stanza.settings.items():
139-
default_stanza.settings[setting_key] = Setting(
140-
key=setting.key,
141-
value=setting.value,
142-
line_number=setting.line_number,
143-
comments=setting.comments.copy(),
144-
)
145-
else:
146-
# Add new stanza
147-
# Create a new stanza with the same type
148-
new_stanza = Stanza(
149-
name=stanza_name,
150-
type=local_stanza.type,
151-
line_number=local_stanza.line_number,
152-
comments=local_stanza.comments.copy(),
153-
blank_lines_after=local_stanza.blank_lines_after,
154-
)
155-
156-
# Copy settings
157-
for setting_key, setting in local_stanza.settings.items():
158-
new_stanza.settings[setting_key] = Setting(
159-
key=setting.key,
160-
value=setting.value,
161-
line_number=setting.line_number,
162-
comments=setting.comments.copy(),
163-
)
164-
165-
default_sorter.stanzas[stanza_name] = new_stanza
166-
else: # Merge mode
167-
for stanza_name, local_stanza in local_sorter.stanzas.items():
168-
if stanza_name not in default_sorter.stanzas:
169-
# Add new stanza
170-
# Create a new stanza with the same type
171-
new_stanza = Stanza(
172-
name=stanza_name,
173-
type=local_stanza.type,
174-
line_number=local_stanza.line_number,
175-
comments=local_stanza.comments.copy(),
176-
blank_lines_after=local_stanza.blank_lines_after,
177-
)
178-
179-
# Copy settings
180-
for setting_key, setting in local_stanza.settings.items():
181-
new_stanza.settings[setting_key] = Setting(
182-
key=setting.key,
183-
value=setting.value,
184-
line_number=setting.line_number,
185-
comments=setting.comments.copy(),
186-
)
187-
188-
default_sorter.stanzas[stanza_name] = new_stanza
189-
else:
190-
# Update values in existing stanza
191-
default_stanza = default_sorter.stanzas[stanza_name]
192-
193-
# Update or add settings from local
194-
for setting_key, setting in local_stanza.settings.items():
195-
default_stanza.settings[setting_key] = Setting(
196-
key=setting.key,
197-
value=setting.value,
198-
line_number=setting.line_number,
199-
comments=setting.comments.copy(),
200-
)
201-
202-
# For multiline handling, we need to read the original local file
203-
# to accurately preserve the structure of multiline values
204-
205-
# Parse the original files to get proper multiline values
206-
local_parsed = _parse_conf_file(local_file)
207-
if default_file.exists():
208-
default_parsed = _parse_conf_file(default_file)
209-
else:
210-
default_parsed = {}
125+
# Create a backup of the default file
126+
default_backup = None
127+
try:
128+
import shutil
129+
130+
default_backup = default_file.with_suffix(
131+
default_file.suffix + ".bak"
132+
)
133+
shutil.copy(default_file, default_backup)
134+
except Exception:
135+
# If backup fails, continue without backup
136+
pass
137+
138+
# Prepare merged configuration
139+
merged_config = self._prepare_merged_config(
140+
local_parsed, default_parsed
141+
)
211142

212-
# Write files safely with fallback
213143
try:
214-
# Write changes to a file
215-
with open(default_file, "w", encoding="utf-8") as f:
216-
# Manual formatting to handle multiline values properly
217-
for stanza_name, stanza in default_sorter.stanzas.items():
218-
# Write stanza header
219-
f.write(f"[{stanza_name}]\n")
220-
221-
# Write stanza settings, preserving multiline structure
222-
for setting_key, setting in stanza.settings.items():
223-
if setting.value is not None:
224-
# Check if this setting came from local and has a multiline structure
225-
if (
226-
stanza_name in local_parsed
227-
and setting_key in local_parsed[stanza_name]
228-
and "\\" in local_file.read_text()
229-
):
230-
# Find the original multiline value in the local file
231-
with open(
232-
local_file, "r", encoding="utf-8"
233-
) as local_f:
234-
local_lines = local_f.readlines()
235-
236-
# Find the key in the original file
237-
multiline_start = None
238-
multiline_lines = []
239-
240-
for i, line in enumerate(local_lines):
241-
if f"{setting_key} =" in line:
242-
multiline_start = i
243-
multiline_lines.append(line.rstrip())
244-
245-
# Collect all continuation lines
246-
j = i + 1
247-
while (
248-
j < len(local_lines)
249-
and multiline_lines[-1].endswith(
250-
"\\"
251-
)
252-
and not local_lines[j]
253-
.strip()
254-
.startswith("[")
255-
):
256-
multiline_lines.append(
257-
local_lines[j].rstrip()
258-
)
259-
j += 1
260-
261-
break
262-
263-
if multiline_start is not None:
264-
# Found multiline value, write it preserving original structure
265-
f.write(multiline_lines[0] + "\n")
266-
for ml_line in multiline_lines[1:]:
267-
f.write(ml_line + "\n")
268-
else:
269-
# Fallback to normal handling
270-
f.write(
271-
f"{setting_key} = {setting.value}\n"
272-
)
273-
else:
274-
# Normal single-line value
275-
f.write(f"{setting_key} = {setting.value}\n")
276-
else:
277-
f.write(f"{setting_key}\n")
278-
279-
# Add blank line between stanzas
280-
f.write("\n")
144+
# Write merged configuration to file
145+
self._write_merged_file(default_file, merged_config, local_content)
146+
147+
# Delete backup if all went well
148+
if default_backup and default_backup.exists():
149+
default_backup.unlink()
281150
except Exception as e:
282-
print(f"Error writing file: {str(e)}")
283-
# Fallback: Copy the local file to default if
284-
# we can't write properly
285-
self._copy_file(local_file, default_file)
151+
# Restore backup if available
152+
if default_backup and default_backup.exists():
153+
try:
154+
shutil.copy(default_backup, default_file)
155+
default_backup.unlink()
156+
except Exception:
157+
pass
158+
# Re-raise the exception
159+
raise e
286160

287161
except Exception as e:
162+
# Log the error
288163
file_result.error = f"Error writing file: {str(e)}"
164+
if self.verbose:
165+
print(f"Error writing file {default_file}: {str(e)}")
166+
import traceback
167+
168+
traceback.print_exc()
169+
170+
def _prepare_merged_config(self, local_parsed, default_parsed):
171+
"""Prepare the merged configuration based on local and default settings.
172+
173+
Args:
174+
local_parsed: Dictionary of stanzas from local file
175+
default_parsed: Dictionary of stanzas from default file
176+
177+
Returns:
178+
Dictionary containing the merged configuration
179+
"""
180+
merged_config = {}
181+
182+
# Start with all settings from default
183+
for stanza_name, settings in default_parsed.items():
184+
merged_config[stanza_name] = settings.copy()
185+
186+
# Add or update settings from local based on merge mode
187+
for stanza_name, settings in local_parsed.items():
188+
# If stanza doesn't exist in merged_config, add it
189+
if stanza_name not in merged_config:
190+
merged_config[stanza_name] = {}
191+
192+
# Replace or merge settings based on mode
193+
if self.mode == MergeMode.REPLACE:
194+
# Replace entire stanza with local version
195+
merged_config[stanza_name] = settings.copy()
196+
else:
197+
# Update settings from local
198+
for setting_key, setting_value in settings.items():
199+
merged_config[stanza_name][setting_key] = setting_value
200+
201+
return merged_config
202+
203+
def _write_merged_file(self, output_file, merged_config, local_content):
204+
"""Write the merged configuration to a file.
205+
206+
Args:
207+
output_file: Path to the output file
208+
merged_config: Dictionary containing the merged configuration
209+
local_content: String containing the local file content for multiline detection
210+
"""
211+
212+
# List of settings to avoid duplicates
213+
written_settings = {}
214+
215+
with open(output_file, "w", encoding="utf-8") as f:
216+
# Process each stanza
217+
for stanza_name, settings in merged_config.items():
218+
# Write stanza header
219+
f.write(f"[{stanza_name}]\n")
220+
written_settings[stanza_name] = set()
221+
222+
# First directly process multiline values from the local file
223+
multiline_settings = set()
224+
225+
# Directly find multiline settings in the local file
226+
stanza_pattern = rf"\[{re.escape(stanza_name)}\](.*?)(?=\n\[|\Z)"
227+
stanza_matches = re.search(stanza_pattern, local_content, re.DOTALL)
228+
229+
if stanza_matches:
230+
stanza_content = stanza_matches.group(1)
231+
# Find individual settings with continuation characters
232+
lines = stanza_content.split("\n")
233+
i = 0
234+
235+
while i < len(lines):
236+
line = lines[i].strip()
237+
238+
# Check if this is a setting line
239+
if "=" in line and not line.startswith("#"):
240+
setting_key, setting_value = line.split("=", 1)
241+
setting_key = setting_key.strip()
242+
243+
# Only process settings that are in our merged config
244+
if setting_key in settings:
245+
# Check if this is potentially a multiline setting
246+
if "\\" in setting_value:
247+
# This is a multiline setting
248+
multiline_lines = [line]
249+
250+
# Collect all continuation lines
251+
j = i + 1
252+
current_line = line
253+
254+
while j < len(lines) and (
255+
"\\" in current_line or j == i + 1
256+
):
257+
current_line = lines[j].strip()
258+
if not current_line.startswith(
259+
"["
260+
): # Not a new stanza
261+
multiline_lines.append(current_line)
262+
j += 1
263+
else:
264+
break
265+
266+
if len(multiline_lines) > 1:
267+
# Write multiline setting
268+
multiline_settings.add(setting_key)
269+
270+
# Write setting line
271+
f.write(
272+
f"{setting_key} = {setting_value.strip()}\n"
273+
)
274+
275+
# Write continuation lines
276+
for ml_line in multiline_lines[1:]:
277+
f.write(f"{ml_line}\n")
278+
279+
written_settings.setdefault(
280+
stanza_name, set()
281+
).add(setting_key)
282+
i = (
283+
j - 1
284+
) # Update index to skip processed lines
285+
286+
i += 1
287+
288+
# Process regular settings
289+
for setting_key, setting_value in sorted(settings.items()):
290+
if setting_key not in written_settings.get(stanza_name, set()):
291+
f.write(f"{setting_key} = {setting_value}\n")
292+
written_settings.setdefault(stanza_name, set()).add(setting_key)
293+
294+
# Add blank lines after each stanza
295+
f.write("\n\n")
289296

290297
def _merge_file(
291298
self, local_file: Path, target_file: Optional[Path] = None

0 commit comments

Comments
 (0)