Repository navigation
ConfigParser writes a superfluous final blank line #77098
Description
| BPO | 32917 |
|---|---|
| Nosy | @mcepl, @stevendaprano, @ambv, @serhiy-storchaka, @titansnow |
| PRs | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2021-12-09.09:39:01.311>
created_at = <Date 2018-02-23.01:29:50.226>
labels = ['type-bug', 'library']
title = 'ConfigParser writes a superfluous final blank line'
updated_at = <Date 2021-12-09.09:39:01.310>
user = 'https://github.com/TitanSnow'bugs.python.org fields:
activity = <Date 2021-12-09.09:39:01.310>
actor = 'iritkatriel'
assignee = 'none'
closed = True
closed_date = <Date 2021-12-09.09:39:01.311>
closer = 'iritkatriel'
components = ['Library (Lib)']
creation = <Date 2018-02-23.01:29:50.226>
creator = 'tttnns'
dependencies = []
files = ['47474']
hgrepos = []
issue_num = 32917
keywords = ['patch']
message_count = 13.0
messages = ['312608', '312615', '312616', '312619', '312622', '313244', '313480', '313513', '313518', '313520', '313521', '313525', '313530']
nosy_count = 5.0
nosy_names = ['mcepl', 'steven.daprano', 'lukasz.langa', 'serhiy.storchaka', 'tttnns']
pr_nums = ['5846']
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32917'
versions = []Reacted by Roman
Activity
TitanSnow commentedon Feb 23, 2018
ConfigParser.write()writes a superfluous final blank line.Example::
The output file 'configparser.ini' will be:: (I added line number)
1 [section1]
2 key = value
3
4 [section2]
5 key = value
6
with a superfluous final blank line.
Compare to
GLib.KeyFile::The output file 'glib.ini' will be:: (I added line number)
1 [section1]
2 key=value
3
4 [section2]
5 key=value
without a superfluous final blank line.
stevendaprano commentedon Feb 23, 2018
Its not a superfluous blank line. It is standard convention for Unix tools to end text files (of which ini files are a kind of text file) with a final newline \n. There are various reasons for this, but it doesn't matter what those reasons are, the important thing is that it is (or at least, ought to be) intentional for the ini file to end with a \n.
It is not an error for the file to end with one (or more) blank lines, regardless of the platform; it is more convenient for all lines to a \n delimiter, rather than making the last line special.
I do not believe there is any good reason to complicate the code and the API with an unnecessary "trim_final_blankline" parameter, but even if there is a good reason, the default would have to be False to remain backwards compatible.
Unless you have a good reason why it should be considered an error for the INI file to end in a blank, this patch should be rejected, and instead we should have an explicit test to ensure that the INI file is always written with a final blank.
(Of course, when *reading* INI files, it should accept them either with or without final blank.)
stevendaprano commentedon Feb 23, 2018
Oh, I forgot... even if it is decided that this trim_final_blankline parameter was desirable, the patch isn't sufficient to be accepted. You would need to also supply documentation and tests.
TitanSnow commentedon Feb 23, 2018
**It's not final newline \n !**
Maybe what I said confused. The "final blank line"
does not mean the "final newline char".
It means an extra blank line after last line.
It might be clearer to represent it in a string::
It has *two* \n at end. I think it should be only one.
TitanSnow commentedon Feb 23, 2018
Patch updated, included documentation and tests.
serhiy-storchaka commentedon Mar 5, 2018
Adding two new parameters to control so tiny detail of the output looks excessive to me. What if just change the default behavior and never output a final blank line?
TitanSnow commentedon Mar 9, 2018
I’m afraid of breaking the backward compatibility.
TitanSnow commentedon Mar 10, 2018
If we treat the original behavior as a bug,
it’s much easier to write a patch
that just changes the default behavior
and never outputs a final blank line.
I have looked through the testsuite of it.
It seems that the original author had knew
the final blank line and treat it as
expect result. I’m not sure about it.
stevendaprano commentedon Mar 10, 2018
On Sat, Mar 10, 2018 at 06:37:28AM +0000, TitanSnow wrote:
Remind me, why do we care about that extra blank line?
Is there a test for it? If so, then it should stay unless the author
agrees it can go, or we get approval on the Python-Dev mailing list.
serhiy-storchaka commentedon Mar 10, 2018
I thought a blank line between sections is just for readability, and no program will break if drop it. Removing the final blank line can harm the readability in the case of sequential writes to the same file (unless we detect the start of the file, that is problematic in the case of non-seekable streams). But I didn't thought that a superfluous final blank line causes any problems. What software has a problem with it?
TitanSnow commentedon Mar 10, 2018
Currently I have not found a program that has problem with
the superfluous final blank line, and I think there won’t be.
Sorry that I have not thought about this.
In this way, the prev patch that adds two new parameters might be better.
Well, though it seems that that extra blank line won’t cause any problem,
it does not look nice I think, and is different with INI files written
by other applications.
TitanSnow commentedon Mar 10, 2018
For the case of sequential writes to the same file,
I think it’s a invalid use case.
The file can be created or modified by
user or other applications, breaking the assume of
ConfigParser. It’s better to have a method to merge
two ConfigParser objects then writes it into the file
at one time.
serhiy-storchaka commentedon Mar 10, 2018
If there is no real problem, I think it is not worth to change this.