jncramp / iniparse

Automatically exported from code.google.com/p/iniparse
Other
0 stars 0 forks source link

Setting new value on an option with continuation lines gives unexpected results #6

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Having an option with a mix of continuation lines and spaces
2. Changing that option to a new value without spaces
3.

What is the expected output? What do you see instead?

Expected:

To get new option with same indentation and without blank lines.

Got:

First line starting on option line (different from original)
New options that exceeded LineContainer.contents have different indentation 
from original.

What version of the product are you using? On what operating system?

iniparse 0.2.4

Please provide any additional information below.

The attached patch adds a test for the expected behavior and fixes 
LineContainer to match the expectations.

Original issue reported on code.google.com by sidnei.d...@gmail.com on 29 Dec 2008 at 9:20

Attachments:

GoogleCodeExporter commented 8 years ago
Updated patch to take care of setting a single value into a LinesContainer 
(inlines 
on OptionLine). Also, setting a value with multiple trailing and leading 
newlines 
strips all but one leading newline and all trailing newlines.

Original comment by sidnei.d...@gmail.com on 30 Dec 2008 at 12:43

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for the bug report and the patch.  I'm traveling at the moment, with very
limited computer access, so I won't be able to test the patch immediately.  
I'll be
back in mid-January - I'll check-in a fix then and update this issue.

Original comment by psobe...@gmail.com on 30 Dec 2008 at 3:52

GoogleCodeExporter commented 8 years ago
Thank you!

Also, I wasn't able to run all the tests: they fail to import test_compat 
because of 
the line 'from test import test_support'. I believe there's a test.py module 
missing?

Original comment by sidnei.d...@gmail.com on 30 Dec 2008 at 11:16

GoogleCodeExporter commented 8 years ago
Regarding the missing test module: some linux distributions don't include 
python's
test suite in the regular python package.  For example, in Fedora the tests are 
in
the "python-test" package.  If the python tests are installed, you should have a
/usr/lib/pythonX.X/test/ directory with over 300 dot-py files in it, one of them
being test_support.py

Original comment by psobe...@gmail.com on 16 Jan 2009 at 2:01

GoogleCodeExporter commented 8 years ago
I've just verified the bug.

Original comment by psobe...@gmail.com on 24 Jan 2009 at 2:51

GoogleCodeExporter commented 8 years ago
I've checked in a simplified version of your patch, with a few changes to 
behaviour.
 Instead of removing empty lines, I've changed it so that empty lines will be
preserved in multi-lines values, both on lookup and assignment.  Can you check 
that
my checkin fixes the issue?

Also, I left out the raw_section_option() function from your patch.  Was it 
there
only because the usual value lookup mechanism was eating empty lines, or was 
there
another reason?

This was a pretty major bug - thanks once again for reporting it, and sorry I 
took so
long to apply the patch.

Original comment by psobe...@gmail.com on 3 Feb 2009 at 4:51

GoogleCodeExporter commented 8 years ago
I just realized why empty lines were being skipped - ConfigParser skips empty 
lines
in multi-line values.  But, Python's test suite does not have a test for that
(otherwise I'd have found out this before the checkin).  So the question is: is 
this
empty line skipping behaviour by design, or is it accidental?  And should 
iniparse be
faithful to ConfigParser in this regard?

Original comment by psobe...@gmail.com on 3 Feb 2009 at 5:27

GoogleCodeExporter commented 8 years ago
> Can you check that my checkin fixes the issue?

Indeed it does. Thanks!

> Also, I left out the raw_section_option() function from your patch.  Was it 
there
only because the usual value lookup mechanism was eating empty lines, or was 
there
another reason?

Yup, it was there because the empty lines were being eaten. I've replaced usage 
of 
the raw_section_option() by direct API usage and it works fine now.

> I just realized why empty lines were being skipped... And should iniparse be
faithful to ConfigParser in this regard?

I would say, yes if you use iniparser's ConfigParser, no if you use the 
iniparser's 
INIConfig. That would, in fact, suit my usage.

Original comment by sidnei.d...@gmail.com on 3 Feb 2009 at 7:16

GoogleCodeExporter commented 8 years ago
> I would say, yes if you use iniparser's ConfigParser, no if you use the 
iniparser's 
> INIConfig. That would, in fact, suit my usage.

That make sense to me too.

Original comment by tim.lauridsen on 7 Feb 2009 at 9:58

GoogleCodeExporter commented 8 years ago
So, the truth is even weirder.  When reading from a file, ConfigParser ignores 
empty
lines in multi-line values; but if an option is set to a value with empty lines,
those are preserved.  I think that behaviour can be preserved without too much
complication - I'll give it a try.

Original comment by psobe...@gmail.com on 7 Feb 2009 at 8:20

GoogleCodeExporter commented 8 years ago
I've implemented backward compatible behaviour when using ConfigParser.  The
implementation was slightly hairy - the __getitem__ logic had to be duplicated, 
since
the original __getitem__ could not know if the access was from a compatibility 
layer
or from the new interface.

Also, the parsing has become a bit more involved.  That parsing function was 
hard to
follow already, and now I've added one more complication.

Original comment by psobe...@gmail.com on 22 Feb 2009 at 7:35