Skip to content

Commit 6f2baff

Browse files
nalundgaardnalundgaard
committed
Fix issues with nested properties
**Problem** I merged #18 a little too soon; It has 3 issues: 1. It doesn't handle comment lines between nested properties 2. It doesn't handle a blank (whitespace) after `=` in the nested property opener like the other forms of properties. 3. The test case was inserted in the wrong location in `eini_tests` module; it should have been in its own test function, and it should have had some error cases. **Solution** 1. Add support for comment lines in between nested properties. 2. Add support for blank after `=` in nested property opener. 3. Refactor and extend the tests for nested properties to cover more ground. **Known Issues** After I merged #18, I noticed this warning: ``` Compiled src/eini_lexer.xrl Parse action conflict scanning symbol blank in state 51: Reduce to properties_nested from property_nested (rule 28 at line 97) vs. shift to state 53, adding right sisters to property_nested. Conflict resolved in favor of shift. ``` It appears that the introduction of nesting has added some ambiguity to the grammar aroud the `blank`, and this change makes it a little worse: ``` Parse action conflict scanning symbol break in state 47: Reduce to single_value from blank (rule 44 at line 121) vs. shift to state 52, adding right sisters to blank. Conflict resolved in favor of shift. Parse action conflict scanning symbol blank in state 58: Reduce to properties_nested from property_nested (rule 30 at line 101) vs. shift to state 61, adding right sisters to property_nested. Conflict resolved in favor of shift. Parse action conflict scanning symbol comment in state 58: Reduce to properties_nested from property_nested (rule 30 at line 101) vs. shift to state 12, adding right sisters to property_nested. Conflict resolved in favor of shift. ``` Now there is some ambiguity around `comment` as well, and this appears to cause an issue parsing INI files with a comment line at the end of a block of nested properties when another (not-nested) key follows. I don't know if we should mark this as a known issue or not, but the trigger case is in a commented unit test in `eini_tests`.
1 parent 6fcbb5d commit 6f2baff

File tree

2 files changed

+113
-19
lines changed

2 files changed

+113
-19
lines changed

‎src/eini_parser.yrl

+15-5
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ Nonterminals
2828
title
2929
property_with_skip_lines
3030
properties property_base property
31-
properties_nested property_nested
31+
properties_nested property_nested_open property_nested
32+
indentation
3233
key_part
3334
values single_value
3435
skip_lines
@@ -88,16 +89,23 @@ property_with_skip_lines -> property : '$1'.
8889
property_with_skip_lines -> property skip_lines : '$1'.
8990

9091
property_base -> key_part '=' values break :
91-
{list_to_atom(value_of('$1')), strip_values('$3')}.
92+
{list_to_atom(value_of('$1')), strip_values('$3')}.
9293

9394
property -> property_base : '$1'.
94-
property -> key_part '=' break properties_nested :
95-
{list_to_atom(value_of('$1')), '$4'}.
95+
property -> property_nested_open properties_nested :
96+
{list_to_atom(value_of('$1')), '$2'}.
97+
98+
property_nested_open -> key_part '=' break : '$1'.
99+
property_nested_open -> key_part '=' blank break : '$1'.
96100

97101
properties_nested -> property_nested : ['$1'].
98102
properties_nested -> property_nested properties_nested : ['$1' | '$2'].
99103

100-
property_nested -> blank property_base : '$2'.
104+
property_nested -> indentation property_base : '$2'.
105+
106+
indentation -> blank : $1.
107+
indentation -> blank skip_lines blank : $1.
108+
indentation -> skip_lines blank : $2.
101109

102110
key_part -> word : '$1'.
103111
key_part -> word blank : '$1'.
@@ -116,6 +124,8 @@ single_value -> '[' : "[".
116124
single_value -> '=' : "=".
117125
single_value -> ']' : "]".
118126

127+
Expect 3.
128+
119129
Erlang code.
120130

121131
-spec value_of({atom(), _Line, TokenChars::string()}) -> TokenChars::string().

‎test/eini_tests.erl

+98-14
Original file line numberDiff line numberDiff line change
@@ -349,20 +349,6 @@ one_section_title_and_one_prop_test_() ->
349349
"[title] \n"
350350
"key1= value1 \n"
351351
)),
352-
%% nested properties: https://docs.aws.amazon.com/credref/latest/refdocs/file-format.html
353-
?_assertEqual({ok, [
354-
{title, [{key1,[{key11,<<"value11">>},{key12,<<"value12">>},{key13,<<"value13">>},{key14,<<"value14">>}]},{key2,<<"value2">>}]}
355-
]},
356-
parse(
357-
"[title]\n"
358-
"key1 =\n"
359-
" key11=value11\n"
360-
" key12 =value12\n"
361-
" key13= value13\n"
362-
" key14 = value14\n"
363-
"key2=value2\n"
364-
)),
365-
366352
%% value has characters which can not used in titles or keys
367353
?_assertEqual({ok, [
368354
{title, [{key1, <<"value1$% '""#!+*=@/:+">>}]}
@@ -461,6 +447,84 @@ binary_two_section_test_() ->
461447
))
462448
]}.
463449

450+
nested_properties_test_() ->
451+
%% https://docs.aws.amazon.com/credref/latest/refdocs/file-format.html
452+
{setup,
453+
fun setup/0,
454+
fun teardown/ 1,
455+
[
456+
?_assertEqual({ok, [
457+
{title,
458+
[{key1, [{key11, <<"value11">>},
459+
{key12, <<"value12">>}]}]}
460+
]},
461+
parse(
462+
"[title]\n"
463+
"key1=\n"
464+
" key11=value11\n"
465+
" key12 =value12\n"
466+
)),
467+
?_assertEqual({ok, [
468+
{title,
469+
[{key1, [{key11, <<"value11">>},
470+
{key12, <<"value12">>},
471+
{key13, <<"value13">>},
472+
{key14, <<"value14">>}]},
473+
{key2, <<"value2">>}]}
474+
]},
475+
parse(
476+
"[title]\n"
477+
"key1 =\n"
478+
" key11=value11\n"
479+
" key12 =value12\n"
480+
" key13= value13\n"
481+
" key14 = value14\n"
482+
"key2=value2\n"
483+
)),
484+
?_assertEqual({ok, [
485+
{title,
486+
[{key1, [{key11, <<"value11">>}]},
487+
{key2, <<"value2">>}]}
488+
]},
489+
parse(
490+
"[title]\n"
491+
"key1= \n"
492+
" key11=value11\n"
493+
"key2=value2\n"
494+
)),
495+
%% with comments in/between nested properties
496+
?_assertEqual({ok, [
497+
{title,
498+
[{key1, [{key11, <<"value11">>},
499+
{key12, <<"value12">>}]},
500+
{key2, <<"value2">>}]}
501+
]},
502+
parse(
503+
"[title]\n"
504+
"key1= \n"
505+
" ; comment in nested properties\n"
506+
" key11 = value11\n"
507+
" ; comment between nested properties\n"
508+
" key12 = value12\n"
509+
"key2=value2\n"
510+
))%,
511+
%% with comments at end of nested properties (TODO Fix this case)
512+
% ?_assertEqual({ok, [
513+
% {title,
514+
% [{key1, [{key11, <<"value11">>},
515+
% {key12, <<"value12">>}]},
516+
% {key2, <<"value2">>}]}
517+
% ]},
518+
% parse(
519+
% "[title]\n"
520+
% "key1= \n"
521+
% " key11 = value11\n"
522+
% " key12 = value12\n"
523+
% "; comment after nested properties\n"
524+
% "key2=value2\n"
525+
% ))
526+
]}.
527+
464528
lex_error_title_test_() ->
465529
{setup,
466530
fun setup/0,
@@ -534,6 +598,26 @@ syntax_error_property_test_() ->
534598
"key;comment=value\n"))
535599
]}.
536600

601+
syntax_error_nested_property_test_() ->
602+
{setup,
603+
fun setup/0,
604+
fun teardown/1,
605+
[
606+
%% no nested properties after open
607+
?_assertMatch({error, {syntax_error, 2, ["syntax error before: ", _]}},
608+
parse(
609+
"[title]\n"
610+
"key1 =\n"
611+
)),
612+
%% no nested properties after open (with following key)
613+
?_assertMatch({error, {syntax_error, 3, ["syntax error before: ", _]}},
614+
parse(
615+
"[title]\n"
616+
"key1=\n"
617+
"key2=value2\n"
618+
))
619+
]}.
620+
537621
dup_title_test_() ->
538622
{setup,
539623
fun setup/0,

0 commit comments

Comments
 (0)