Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Feature request: Allow comments #6

Open
andyuk opened this issue Oct 25, 2011 · 9 comments
Open

Feature request: Allow comments #6

andyuk opened this issue Oct 25, 2011 · 9 comments

Comments

@andyuk
Copy link

andyuk commented Oct 25, 2011

Thanks for this nice module. I have a little feature request.

Would you be able to add support for comments? I think the standard for .ini file is a semi-colon isn't it?

  • Andrew
@shockie
Copy link
Owner

shockie commented Oct 27, 2011

Hi,

Thanks for the feature request, I'm not really sure why you want to parse the comments. If they have valuable information, why don't you make them a variable? Also how can you identify a comment? values have keys and sections but a comment has no key to identify them.

@andyuk
Copy link
Author

andyuk commented Oct 27, 2011

I think you have mis-understood me. I don't want comments to be parsed. It looks like semi-colon quotes are not supported. Am I missing something?

I just want to be able to do something like:

; here is a comment that explains about this setting
key=value

@shockie
Copy link
Owner

shockie commented Oct 31, 2011

Ah yes, i've mis-understood you. Comments are left out of the parsed data, so you can add comments in the .ini file. i've added a test case in the test suite to test for comment parsing.

@shockie shockie closed this as completed Oct 31, 2011
@andyuk
Copy link
Author

andyuk commented Nov 1, 2011

I've just tested this again and found there is a bug (which is why I reported this in the first place).

[category]
; here is a comment that explains about this setting
key=value

this will produce the json:

{
category {},
key: value
}

it should be:

{
category {
  key: value
  }
}

Thanks for looking at this.

Andrew

@shockie shockie reopened this Nov 2, 2011
@shockie
Copy link
Owner

shockie commented Nov 2, 2011

Hmm strange,

I'm using node 0.4.12 and node-iniparser 1.0.4 and can't reproduce this problem. Can you run this piece of code and tell me what it returns? (example.ini is you piece of ini, you've mentioned earlier and run this code in the root of your git checkout)

var iniparser = require('./lib/node-iniparser');
var config = iniparser.parseSync('./test/files/example.ini');
console.log(config.category.key);

What version are you using, and which node version?

@andyuk
Copy link
Author

andyuk commented Nov 4, 2011

That example you provided worked fine. I've found the issue. It's the blank lines that causes the problem. Try the following example. That will return "undefined" for config.category.key.

[category]
; comment

key=value

@shockie
Copy link
Owner

shockie commented Nov 9, 2011

I know what you mean but if it's a bug, I don't know. based on the article on wikipedia http://en.wikipedia.org/wiki/INI_file, I define the blank line as a break of the section. How can I otherwise determine if the section has ended, double white line?

@andyuk
Copy link
Author

andyuk commented Dec 2, 2011

Hi again,

We've just had an issue with our server admin not realising that double white space lines are not allowed.

It's great that you are following the specs so closely, however in this case, this feels like a bug. A section should be indicated by the start of a new [section].

eg.

key=value
[section]
section=true


section=still-true

That would mean it wouldn't be possible to add more keys and values that are not in a section if a section has already been started. I think that's OK since those keys/values should be at the top anyway.

What do you think?

Thanks for your efforts on this library by the way, it's been really helpful for us. :)

Cheers,
Andrew

@shockie
Copy link
Owner

shockie commented Dec 5, 2011

Good point, as more requests are coming in on parsing different types of ini formats, I think it's time to make the parsing more configurable. ie set config flags like allowDoubleWhite:true on the constructor or make different parsing 'engines'. Anyhow it's time to set a standard.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants