Introduce AST + validation errors about specification

[randstr]

@jedmao Will do, thanks.

Another point is that in most Unix systems filenames can contain any character except / and NUL. Including control characters, which no sane person would ever use. But we shouldn't impose artificial restrictions either.

[jednano]

1. I don't think it's intended to solve a problem per se, but to open up the possibility of all the various things you can do with an AST, especially for people who want to use the AST in such a way that has nothing to do with EditorConfig, specifically. For example, I might want to have my own configuration file that followed the same INI format, but all I need is the parser and not the rest.
2. It would not at all change the interface between cores and their respective plugins.
3. The intermediate state, in this context, would be the AST. Currently, the only thing close to this would be whatever data structure you're holding in memory, we're not exactly sharing this data with anyone. Rather, the js core does expose an API that gives you a simple record of key-value pairs, but not the full AST.

[xuhdev]

ConfigParser defines the syntax as:

A configuration file consists of sections, each led by a [section] header, followed by key/value entries separated by a specific string (= or : by default 1). By default, section names are case sensitive but keys are not 1. Leading and trailing whitespace is removed from keys and values. Values can be omitted, in which case the key/value delimiter may also be left out. Values can also span multiple lines, as long as they are indented deeper than the first line of the value. Depending on the parser’s mode, blank lines may be treated as parts of multiline values or ignored.

Configuration files may include comments, prefixed by specific characters (# and ; by default 1). Comments may appear on their own on an otherwise empty line, possibly indented. 1

Do you think it's OK if we refer to this paragraph in some way?

[cxw42]

In the voting, would you please include an option that a core is not required to produce an AST, but shall produce the standard one if it produces any at all? That would permit cores to be updated to produce an AST gradually, as individual projects needed an AST.

Edit Thinking about it some more, could we have more detail before the vote?

• How will the AST be specified? BNF?
• Will there be a standard serialization?
• Will there be a standard API that cores will be required to implement? E.g., will the JS core become or include an NPM module that provides certain functions?
  • If so, how will we handle the differences between programming languages and their capabilities? I am thinking of OpenCV as an example - the Py and C++ APIs are close, but are different enough that they have to be documented separately.
• Will AST output be a feature to be added to existing cores, or to a separate program? E.g., editorconfig --output-ast=json <filename> vs. editorconfig-ast --json <filename>.

In my personal opinion, I think it is reasonable to define a standard option that, when given to a core that supports it, will output an AST in a standardized serialization format. I think core support for the AST should be optional. I do not think standardizing an API is advisable.

Basically, let the projects that want an AST have one without burdening any others :D .

Edit 2 I think this issue should be moved to the main editorconfig repo for greater community visibility.

[jednano]

1. There's no one exact specification of BNF; otherwise, I could see it being extremely valuable to have one BNF file across all cores that is tested only once (i.e., does not need to be tested per core). Please prove me wrong!
2. A standard serialization of what, exactly? I would expect the rule values to be parsed into primitive types like true, false, numbers and everything else is just a string.
3. I personally think it would be ideal to standardize the API and consolidate the documentation across all cores. This will obviously introduce breaking changes, but I don't see that as an issue, as those using the older APIs still work as they do today.
4. Great question! Currently, the editorconfig command just spits out the resolved configuration for a particular file. I don't see this changing at all and I don't see a reason (yet) to spit out JSON from the editorconfig command, because if they want JSON then they can get it directly from the API.

I was thinking the JS core would be published as @editorconfig/cli, which would depend on @editorconfig/core, which would depend on @editorconfig/parse. This separation means users can install just the parser for getting AST or just the core for getting the resolved configuration without any need for a CLI command.

That said, if a core doesn't want to provide an AST, they can stay on the current version path without introducing any breaking changes.

I need to jump on a ✈️

[jednano]

I see what you mean on the serialization point. Being a JS/TS developer, I'm not sure if I'm biased to prefer JSON, but I could also see a valid argument for the YML format to keep things agnostic here.

I agree with your idea to go down this path on the js core and then let things settle before pitching it as a standard for other cores. I'll go down that road. Thanks for listening!

[florianb]

I'm confused by the statement that the INI file format specification still seems not to exist. What do we call this then? I'm just trying to understand what's missing, because I think we want the same thing.

I am not claiming any opinion for the board - i say the INI file format is informal and only defined by different parser implementations. I once wrote a ini-parser and stumbled over the problem finding a "specification" which is independent of implementations.

(If considering depending on @xuhdevs citation "Depending on the parser’s mode, blank lines may be treated as parts of multiline values or ignored" - i'd appreciate if the specification would become unambiguously.)

The specification should probably also describe the editorconfig-specific inheriting behaviour.

And regarding the serialization issue i'd like to add that i'd vote for a JSON-compatible format, including all serialization-formats allowing to represent the JSON-subset. That would offer the possibility to offer for each implementation the most natural format.
Once a specific serialization is implemented it should be done.

I'd like to help @jedmao - so if you'd like any helping hand for any stuff, give me a hint.

[jednano]

@florianb I'm running into the exact same issues writing my 2nd INI parser now, so I feel you.

I agree that we need more clarification and less ambiguity. The ambiguity is killing me!

I don't think the specification for the INI parsing should say anything at all about the inheriting behavior, as I can parse INI files all day long without any concept of merging. That's an EditorConfig-specific thing that lies outside of the INI parser itself, which should be useful for various non-EditorConfig implementations.

I support serialization in the JSON format. All I need to do is JSON.stringify on my end, but it also makes sense in other contexts.

@florianb I appreciate the offer to help. I've invited you to the EditorConfig Slack workspace. I'm not in love with my first INI parser because of all the configuration options that are required. I'd rather just have one EditorConfig mode and stick with that. Also, I'd rather have a formal grammar and a full suite of tests writtten in my implementation's native language, before running any agnostic tests like the CMake ones. It's just easier to mentally track and iterate (not a fan of CMake).

So that's exactly what I did. I have a wip branch I'm actively working on. I have some long flights and lonely nights ahead of me (in Berlin -- hi @florianb), so I'll probably make some good progress on this one.

I was hoping that once that project settles that I can take a step back and see what official "specification" I can extract from it.

Another thing that really doesn't sit well with me is this EditorConfig specification statement:

EditorConfig files use an INI format that is compatible with the format used by Python configparser Library.

Not sure if that's a blanket statement or if all the cores are actually expected to follow that specification wholly or not. I thought I saw some ambiguity in that specification too, but the one feature that makes no sense to me is allowing ] inside a section header? In what scenario would this be a real thing that somebody wants? Should we really go out of our way to support these edge cases?

[florianb]

I support that idea and i would combine it with a formal specification of that obscure ini-file-format.

[jednano]

@florianb my initial issue (now edited) was also to decouple the obscure INI file format from the EditorConfig-specific implementation, but in looking at the current specification site, I think we're already doing that? How would you separate it more?

[cxw42]

@jedmao I think the idea in general is not unreasonable. I have some questions for the sake of understanding.

• What problem(s) would this solve?
• Would this change the interface between the core and the plugins?
• What is the "intermediate state" to which you refer? The contents of the .editorconfig files?

Thanks!

[jednano]

A proper AST would come complete with line numbers and columns for each defined rule, so we could throw nice errors that tell people what they're doing wrong. We don't really do any validation errors at this point. I don't know I would call this a problem, so much as a feature.

[florianb]

@florianb my initial issue (now edited) was also to decouple the obscure INI file format from the EditorConfig-specific implementation, but in looking at the current specification site, I think we're already doing that? How would you separate it more?

Excuse me please - I think I've been misleading. I tried to say that i support making formal aspects of parsing editorconfig-files more specific. And i would welcome if we also could provide a formal specification for the ini-format in general, since a specification still seems not to exist - and a reference to a parser-implementation is not enough to implement a parser on its own.

[jednano]

I'm confused by the statement that the INI file format specification still seems not to exist. What do we call this then? I'm just trying to understand what's missing, because I think we want the same thing.

[jednano]

I think we can merge that with what we already have on the specification site, but it might read better as a bulleted list.

[xuhdev]

@jedmao OK. Should we start a voting about this? I feel this is on the verge of requiring a voting or not.

[jednano]

Yeah. I suppose this is voteworthy.

[cxw42]

@jedmao Safe travels!

I understand your initial interest is in an API providing (access to) an AST, and not in an external format for AST data. I know that API design can take some experimentation and practical experience. I would suggest developing the JS core as you describe without worrying about standardization. Since the external behaviour of the JS core won't change, there is no need to update the EditorConfig specifications yet. Once you have evolved an API/AST that you are satisfied with, you could then propose it as a standard for other cores to implement if they wished to.

Advantages:

• you could develop at your own pace, without needing EditorConfig votes for changes
• EditorConfig would benefit from your experience designing and implementing the first iteration of the API
• The EditorConfig team would be able to see some of the other use cases of the parser/AST when settling on the eventual EC standard API/AST.

What say you?

Regarding serialization, I was thinking of a serialization of whatever the AST turned out to be. I agree that most of the values would be strings. The serialization could be JSON, YML, XML, or any other easy-to-load format.

[cxw42]

@jedmao Sounds good! I'll let you know if I get to a prototype of a more flexible testing system.

[randstr]

Another thing that really doesn't sit well with me is this EditorConfig specification statement:

EditorConfig files use an INI format that is compatible with the format used by Python configparser Library.

Not sure if that's a blanket statement or if all the cores are actually expected to follow that specification wholly or not. I thought I saw some ambiguity in that specification too, but the one feature that makes no sense to me is allowing ] inside a section header? In what scenario would this be a real thing that somebody wants? Should we really go out of our way to support these edge cases?

Isn't that required for [seq] globs? Unless I misunderstood you.

I agree that a (formal) specification shouldn't be defined through another implementation (ConfigParser in this case). It's just confusing. For example are : separators part of the specification? ConfigParser supports it, editorconfig-core-c does as well, but the EditorConfig specification is completely silent on that point (so presumed to not be part of EditorConfig syntax).

@jedmao BTW I'm going through the same process while experimenting with a pure Lua implementation for the Lua core, with similar goals and many of the same difficulties.

[jednano]

@randstr I forgot about the [seq] glob syntax. Thatt makes sense!

Please email me at jedmao at outlook dot com so I can add you to the EditorConfig slack workspace.