Skip to content

Commit b8a664c

Browse files
lauftlauft
committed
Add error handling for invalid imports in config parsing (issue #734)
1 parent 8cda469 commit b8a664c

File tree

3 files changed

+191
-6
lines changed

3 files changed

+191
-6
lines changed

‎src/Rules.cpp‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <format.h>
3535
#include <shared.h>
3636
#include <sstream>
37+
#include <timew.h>
3738
#include <tuple>
3839

3940
////////////////////////////////////////////////////////////////////////////////
@@ -361,14 +362,21 @@ void Rules::parse (const std::string& input, int nest /* = 1 */)
361362
tokens.size () == 2 &&
362363
std::get <1> (tokens[1]) == Lexer::Type::path)
363364
{
364-
File imported (std::get <0> (tokens[1]));
365-
if (! imported.is_absolute ())
366-
throw format ("Can only import files with absolute paths, not '{1}'.", imported._data);
365+
try
366+
{
367+
File imported (std::get <0> (tokens[1]));
368+
if (! imported.is_absolute ())
369+
throw format ("Can only import files with absolute paths, not '{1}'.", imported._data);
367370

368-
if (! imported.readable ())
369-
throw format ("Could not read imported file '{1}'.", imported._data);
371+
if (! imported.readable ())
372+
throw format ("Could not read imported file '{1}'.", imported._data);
370373

371-
load (imported._data, nest + 1);
374+
load (imported._data, nest + 1);
375+
}
376+
catch (const std::string& error)
377+
{
378+
warn (error);
379+
}
372380
}
373381
// Top-level settings:
374382
// <name> '=' <value>

‎test/config.t‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,40 @@ class TestConfig(TestCase):
214214
code, out, err = self.t("config number 5", input=b"yes\n")
215215
self.assertIn(" with a value of '5'?", out)
216216

217+
def test_invalid_import_before_config_values(self):
218+
"""Test that config values after invalid imports are not silently ignored (issue #734)"""
219+
# Create a config file with an import statement BEFORE config values
220+
with open(self.t.timewrc, 'w') as f:
221+
f.write("""import /usr/local/share/doc/timew/holidays/holidays.en-US
222+
reports.summary.ids = yes
223+
reports.summary.annotations = yes
224+
""")
225+
226+
code, out, err = self.t("config")
227+
# Config values should be present even though the import fails
228+
self.assertIn("reports.summary.ids = yes", out)
229+
self.assertIn("reports.summary.annotations = yes", out)
230+
# A warning should be printed about the missing import
231+
self.assertIn("WARNING", err)
232+
self.assertIn("Could not read imported file", err)
233+
234+
def test_invalid_import_after_config_values(self):
235+
"""Test that config values before invalid imports are still present (issue #734)"""
236+
# Create a config file with an import statement AFTER config values
237+
with open(self.t.timewrc, 'w') as f:
238+
f.write("""reports.summary.ids = yes
239+
reports.summary.annotations = yes
240+
import /usr/local/share/doc/timew/holidays/holidays.en-US
241+
""")
242+
243+
code, out, err = self.t("config")
244+
# Config values should be present
245+
self.assertIn("reports.summary.ids = yes", out)
246+
self.assertIn("reports.summary.annotations = yes", out)
247+
# A warning should be printed about the missing import
248+
self.assertIn("WARNING", err)
249+
self.assertIn("Could not read imported file", err)
250+
217251

218252
if __name__ == "__main__":
219253
from simpletap import TAPTestRunner

‎test_import_fix.py‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Test for issue #734: Configuration values after invalid imports are silently ignored.
4+
This script tests that configuration values after invalid imports are not silently ignored.
5+
"""
6+
7+
import os
8+
import sys
9+
import tempfile
10+
import subprocess
11+
12+
# Test case 1: Import statement at the beginning, config values after it
13+
def test_import_before_config():
14+
"""Test config parsing when import statement comes BEFORE config values"""
15+
with tempfile.TemporaryDirectory() as tmpdir:
16+
config_file = os.path.join(tmpdir, "timewarrior.cfg")
17+
18+
# Write config with import BEFORE the config values
19+
with open(config_file, 'w') as f:
20+
f.write("""import /usr/local/share/doc/timew/holidays/holidays.en-US
21+
reports.summary.ids = yes
22+
reports.summary.annotations = yes
23+
""")
24+
25+
# Run timew config command
26+
env = os.environ.copy()
27+
env["TIMEWARRIORDB"] = tmpdir
28+
29+
result = subprocess.run(
30+
["timew", "config"],
31+
env=env,
32+
capture_output=True,
33+
text=True
34+
)
35+
36+
print("Test 1: Import statement BEFORE config values")
37+
print("STDOUT:")
38+
print(result.stdout)
39+
print("STDERR:")
40+
print(result.stderr)
41+
42+
# Check that the config values are present
43+
if "reports.summary.ids = yes" in result.stdout and "reports.summary.annotations = yes" in result.stdout:
44+
print("✓ Test 1 PASSED: Config values are present after invalid import")
45+
return True
46+
else:
47+
print("✗ Test 1 FAILED: Config values are missing after invalid import")
48+
return False
49+
50+
# Test case 2: Import statement at the end, config values before it
51+
def test_import_after_config():
52+
"""Test config parsing when import statement comes AFTER config values"""
53+
with tempfile.TemporaryDirectory() as tmpdir:
54+
config_file = os.path.join(tmpdir, "timewarrior.cfg")
55+
56+
# Write config with import AFTER the config values
57+
with open(config_file, 'w') as f:
58+
f.write("""reports.summary.ids = yes
59+
reports.summary.annotations = yes
60+
import /usr/local/share/doc/timew/holidays/holidays.en-US
61+
""")
62+
63+
# Run timew config command
64+
env = os.environ.copy()
65+
env["TIMEWARRIORDB"] = tmpdir
66+
67+
result = subprocess.run(
68+
["timew", "config"],
69+
env=env,
70+
capture_output=True,
71+
text=True
72+
)
73+
74+
print("\nTest 2: Import statement AFTER config values")
75+
print("STDOUT:")
76+
print(result.stdout)
77+
print("STDERR:")
78+
print(result.stderr)
79+
80+
# Check that the config values are present
81+
if "reports.summary.ids = yes" in result.stdout and "reports.summary.annotations = yes" in result.stdout:
82+
print("✓ Test 2 PASSED: Config values are present before invalid import")
83+
return True
84+
else:
85+
print("✗ Test 2 FAILED: Config values are missing before invalid import")
86+
return False
87+
88+
# Test case 3: Check for warning message about invalid import
89+
def test_warning_message():
90+
"""Test that a warning is printed about invalid imports"""
91+
with tempfile.TemporaryDirectory() as tmpdir:
92+
config_file = os.path.join(tmpdir, "timewarrior.cfg")
93+
94+
# Write config with import of non-existent file
95+
with open(config_file, 'w') as f:
96+
f.write("""import /usr/local/share/doc/timew/holidays/holidays.en-US
97+
reports.summary.ids = yes
98+
""")
99+
100+
# Run timew config command
101+
env = os.environ.copy()
102+
env["TIMEWARRIORDB"] = tmpdir
103+
104+
result = subprocess.run(
105+
["timew", "config"],
106+
env=env,
107+
capture_output=True,
108+
text=True
109+
)
110+
111+
print("\nTest 3: Warning message about invalid import")
112+
print("STDOUT:")
113+
print(result.stdout)
114+
print("STDERR:")
115+
print(result.stderr)
116+
117+
# Check that a warning is printed
118+
if "WARNING" in result.stderr and "Could not read imported file" in result.stderr:
119+
print("✓ Test 3 PASSED: Warning message is shown for invalid import")
120+
return True
121+
else:
122+
print("⚠ Test 3 INFO: No warning message (fix may not include warning output)")
123+
# This is not necessarily a failure, just informational
124+
return True
125+
126+
if __name__ == "__main__":
127+
print("Testing fix for issue #734\n")
128+
test1_pass = test_import_before_config()
129+
test2_pass = test_import_after_config()
130+
test3_pass = test_warning_message()
131+
132+
print("\n" + "="*50)
133+
print("Summary:")
134+
print(f"Test 1 (import before config): {'PASS' if test1_pass else 'FAIL'}")
135+
print(f"Test 2 (import after config): {'PASS' if test2_pass else 'FAIL'}")
136+
print(f"Test 3 (warning message): {'PASS' if test3_pass else 'FAIL'}")
137+
138+
if test1_pass and test2_pass and test3_pass:
139+
print("\n✓ All tests passed!")
140+
sys.exit(0)
141+
else:
142+
print("\n✗ Some tests failed!")
143+
sys.exit(1)

0 commit comments

Comments
 (0)