Skip to content
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

Comments support #77

Open
singleton11 opened this issue Feb 16, 2017 · 11 comments
Open

Comments support #77

singleton11 opened this issue Feb 16, 2017 · 11 comments
Labels
component: decoder Related to parsing in `toml.load` syntax: comments Related to comments type: feature A self-contained enhancement or new feature

Comments

@singleton11
Copy link

singleton11 commented Feb 16, 2017

I want to be able to load some toml with comments and add some key = value to this file and save with comment saving

@uiri
Copy link
Owner

uiri commented Feb 17, 2017

Patches (Pull Requests) welcome!

I recommend saving it in some key value pairs which your code then ignores. For example:

[comments]
logging = "The logs are rotated every 2 weeks"

@samvasko
Copy link
Contributor

or just append the changes

@ryanhiebert
Copy link
Collaborator

In order to implement this, we'll have to live with some ambiguity, or move to using ordered dicts everywhere. Consider this TOML file.

# This is my TOML file

[mytable]
# My table is great

# Make it greater
greater = true

greatest = true  # Make it the greatest

There are, sort of, two types of comments. Inline comments that appear in the same line as a line of code, and full-line comments that simply appear near their code.

Because the tables are unordered, and the order may change when dumping the tables, the "best" way to preserve the comments is to relate them to a line of configuration. Inline comments are simple to do this way: just attach them to whatever was on the line with them. Whole line or multi-line comments are ambiguous, though, as they may better attach to the parent scope, or to the following configuration. Without preserving even more of the file, especially the ordering, there's not a great way to deal with this ambiguity.

We can make a choice about which is the Right Way for whole-line comments to be considered, but I'm not sure which way it is. I think I'd be inclined toward it being associated with the following item if it exists, otherwise with the parent scope.This association is guaranteed to be wrong some of the time, though, without a good solution. The only real fix for that is to make it not matter whether we get it right or not, by making sure that the keys end up staying where they need to.

@uiri
Copy link
Owner

uiri commented Apr 11, 2017

So, my initial reaction (per my earlier response in this issue) is that preserving comment information is overkill for almost all users of load/loads. For that reason, I don't think that load/loads should preserve comment information (at least, not by default, and not via a boolean flag).

However, after looking at kennethreitz/pipenv#212, and what pipenv does to some TOML files, I think that there should be a facility in the library to allow its users to easily preserve comment information (or other information about a given piece of TOML).

@ncoghlan
Copy link

ConfigObj is a project that provides comment-preserving editing of INI-files (unlike the standard library's configparser module), so that may provide some useful inspiration for adding a comment-preserving editing mode here: https://github.com/DiffSK/configobj/blob/master/src/configobj/__init__.py

However, I agree that it makes sense for comment-preserving to be an opt-in behaviour - it's only needed when tools want to support automated editing of the config file, and that's the exception rather than the rule.

@uiri uiri mentioned this issue Sep 7, 2017
@AlJohri
Copy link

AlJohri commented Sep 13, 2017

move to using ordered dicts everywhere

any chance this would work OOB in Python 3.6?

@amjith
Copy link
Contributor

amjith commented Sep 13, 2017

With the latest changes committed to master it will always use the same datastructure that was passed in. So you can pass in an OrderedDict and the result will ordered as expected.

@yjqiang
Copy link

yjqiang commented Oct 29, 2018

Maybe we can generate 'key_comment' with 'key', and then we can make it. By the way, Python 3.7 – Dictionaries now ordered.

@yjqiang
Copy link

yjqiang commented Dec 2, 2018

So, my initial reaction (per my earlier response in this issue) is that preserving comment information is overkill for almost all users of load/loads. For that reason, I don't think that load/loads should preserve comment information (at least, not by default, and not via a boolean flag).

However, after looking at kennethreitz/pipenv#212, and what pipenv does to some TOML files, I think that there should be a facility in the library to allow its users to easily preserve comment information (or other information about a given piece of TOML).

So is this finished now? Or I think I can try. I decided to add sth like "key_comment": "# sth" besides "key": "value" to make it.

@uiri
Copy link
Owner

uiri commented Dec 2, 2018

Any open issue is up for grabs for anyone to tackle. Please do open up a pull request if you have a patch you'd like to see merged in.

Unfortunately, I have not yet made any progress on this particular issue.

This was referenced Apr 6, 2019
csadorf added a commit to aiidateam/aiida-core that referenced this issue Mar 6, 2020
Also update the current file to match the exact format of the
automatically generated file. Unfortunately that means the removal of
the comment, because toml currently does not support preservation of
comments, see here: uiri/toml#77
@pradyunsg pradyunsg added type: feature A self-contained enhancement or new feature component: decoder Related to parsing in `toml.load` syntax: comments Related to comments and removed user-customization labels Apr 20, 2022
@vcidst
Copy link

vcidst commented Oct 24, 2024

Is this still an open issue? From the linked pull requests I see that something has been merged to the same effect but I cannot find it in the documentation. #249 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: decoder Related to parsing in `toml.load` syntax: comments Related to comments type: feature A self-contained enhancement or new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants