-
Notifications
You must be signed in to change notification settings - Fork 628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iniconf: some more adjustments for parsing TOML #4099
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4099 +/- ##
==========================================
+ Coverage 85.72% 85.85% +0.12%
==========================================
Files 239 239
Lines 56961 58629 +1668
==========================================
+ Hits 48827 50333 +1506
- Misses 8134 8296 +162 ☔ View full report in Codecov by Sentry. |
@masatake Any opinion on this pull request? |
Sorry for the late reply. My primary question is whether this pull request intends a bug fix or a hack for adjusting the Iniconf parser to TOML. We provide a PEG-based TOML parser. So modifying the Iniconf parser for supporting TOML is not a standard practice. This is my baseline to evaluate this pull request. On the other hand, the TOML parser doesn't work well (#4096). As you were afraid, I'm not familiar with PEG enough so I have not fixed the simple bug yet. So I don't ask you to use the TOMP parser. Hacking Iniconf for supporting TOML makes sense. But this is still a hack. So I have some requests. Please, see my review comments. |
Units/parser-iniconf.r/simple-pythonLoggingConfig.d/expected.tags
Outdated
Show resolved
Hide resolved
You are right, I want to use this parser for parsing TOML in Geany so it's kind of hack. On the other hand, it's not such a hack - there's no formal ini specification so I think what I propose is still a valid ini. The way I understand ini files intuitively is:
So in fact, I believe the key name it could be any character sequence (with stripped leading and trailing spaces). And for the section name it should be the first In any case, no problem to add the changes you propose. |
For TOML array of tables like [[array]] this change creates the tag [array] instead of the previously created tag [array
This allows TOML "key in double quotes" = 1 'key in single quotes' = 2
4fc5ddb
to
a0cc4df
Compare
Thank you for updating. I have one more request to change. See my comment.
I see. However, the updated version of this pull request looks more understandable. |
Thank you. |
This patch: - improve parsing of TOML [[arrays_of_tables]] - adds support for 'single quoted keys' = true and "double quoted keys" = true See universal-ctags/ctags#4099
This patch: - allows . and - in keys - improve parsing of TOML [[arrays_of_tables]] - adds support for 'single quoted keys' = true and "double quoted keys" = true See universal-ctags/ctags#4099
These two patches:
[[arrays_of_tables]]
'single quoted keys' = true
and"double quoted keys" = true
See the individual commits for more details.