Open GoogleCodeExporter opened 8 years ago
Thanks for the feedback, but actually I believe such changes may be better
implemented in a fork. The gcfg package is for parsing files with the gcfg
syntax (which is based on that for git-config); I haven't looked into the
syntax of python config files in detail, but it seems likely that there will be
numerous differences / incompatibilities in addition to the two you've created
issues for (I'd be surprised if case sensitivity, the set of valid characters,
escaping/quoting, etc, were all entirely equivalent). On the other hand, the
source code of gcfg may be a good base for creating a parser for such files.
Even though forking creates a bit of code duplication, this makes it explicit
that the two packages are for parsing different formats. (Please use a name
other than gcfg for the fork, to prevent confusion regarding what is valid gcfg
format.)
Perhaps some of the format differences could be handled by relaxing the rules
for the gcfg syntax as your patches suggest, but trying to support a
combination of a variety of syntaxes in a single package is likely impossible
in the long run. (Just picking which of the many INI-style formats to include /
exclude would need to be somewhat arbitrary.) Even assuming it worked, it makes
describing (understanding) and testing the accepted format(s) more complicated.
(Note that your patches didn't include the changes for the documentation and
the tests -- I usually look at those before checking the implementation. Even
though "go test" validates the output of the examples so it works as a kind of
test, I believe those should be there in addition to, not instead of the
primary tests.) I generally prefer less variations to more (like the Go
language itself; instead of having "for", "do", "while", "until", there's only
"for" -- less is more), and prefer to only change things from the git-config
syntax when there is clear benefit. Adding your patches would create additional
variations (':' vs '=', '_' vs '-') that could only bring confusion without any
specific additional value when using the package to parse gcfg files. (Ideally
I'd even prefer if there was only one way of commenting, but making that change
didn't seem to provide sufficient benefit.)
On the gcfg side, there is room to improve the code structure to make it easier
to reuse it for similar formats with less code duplication, so I'll leave this
issue open for tracking that. If you accept my advice and create a fork, and
decide to make it public, please share your repo URL, it will help me in
validating the revised structure.
Original comment by speter....@gmail.com
on 23 Jan 2015 at 12:26
Issue 13 has been merged into this issue.
Original comment by speter....@gmail.com
on 23 Jan 2015 at 12:28
Original issue reported on code.google.com by
michael....@canonical.com
on 22 Jan 2015 at 3:25Attachments: