My personal opinion is that making your own custom config file format sends you immediately to some ring of programming hell. Let's call it YAYAML :)

Anyway, since we've got it and I'm sure someone will tell me that we should have it and we can't use YAML or XML, I think we do need a generator.

Here is a first pass at what is probably a rather piss poor version of one.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JacobSingh’s picture

Status: Active » Needs review
FileSize
1.73 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

mikey_p’s picture

Just out of curiosity, do you have a potential use case for this?

JacobSingh’s picture

Yeah, trying to make a packaging script which will modify info files based on some user customizations.

It could be in contrib, but if you make up YAYAML, I think it should have a parser and a printer, and so I think it should be in core. Really though, I think we should switch to YAML and use a 3rd party parser / generator.

-J

mikey_p’s picture

Well, okay, I've dug out the original issue for drupal_parse_info_file(): #132018: Add .info files to themes (and improve/clean up the theme system). The discussion regarding YAML starts here.

While it's not fun to read through that issue, there was less opposition to YAML there than I remembered. The main arguments against YAML seemed to be:

  • A YAML parser would be an order of magnitude larger than drupal_parse_info_file().
  • Whether or not YAML would really be easier to read than the current array style syntax.
  • Dries gave YAML a -1 and ruled it out. He did not rule out XML however.

I've thought about this before, and done some basic work with YAML. The most prominent YAML parser is spyc and it comes in at almost 1000 lines and 28KB, so argument 1 has some validity. Also, there are a few issues with spyc, for example, it is not currently NOTICE free. It is also used by: symfony (1.0 branch), livepipe, guesswork, andromeda, akelos, and probus.

Argument 2 is so subjective that discussing it would most likely bring no value. (XML is perhaps more debatable, but still...) Without some testing, I don't know how we could answer this conclusively.

#3 ? It would be great to see Dries weigh in before starting?

One of the biggest issues I see, is if the above concerns are address/considered acceptable, do we roll our own YAML parser, based on spyc, write our own from scratch, or just include spyc as is? Overall I'd have to say, that while the library is large, I'd be against modifying it, or including some form of YAML-lite. It's tempting to roll our own, but our time could perhaps be better spent fixing the small issues in it, and then concentrating on other performance issues? (Unless this happens to be a major performance issue).

JacobSingh’s picture

Wow, nice research!

I don't really want to get into the argument of inventing our own vs using an existing standard / toolkit because I guess it's already done, just giving my 2c that unless it's a massive performance difference, using an existing tool wins in my book.

chx’s picture

Dries http://drupal.org/node/132018#comment-534330 argument was ". It's not consistent with the module .info files, which I think is important." There was no other real argument against YAML just saying "it's a mess" when it's not. walkah and I were in favor and dww was at least not against.

Bronsa’s picture

Status: Needs work » Needs review
FileSize
1.11 KB

Status: Needs review » Needs work

The last submitted patch, common.inc_.diff, failed testing.

Bronsa’s picture

FileSize
1.37 KB

sorry, the previous patch had been created using diff, this should be ok, i created it using cvs -up

cwgordon7’s picture

Status: Needs work » Needs review
cwgordon7’s picture

This is going to need accompanying unit test files. Please read the documentation, or ask on IRC (#drupal-gci or #drupal-contribute) if you need help. Thanks!

Bronsa’s picture

FileSize
2.15 KB

Here it is the unit test

Status: Needs review » Needs work

The last submitted patch, test.patch, failed testing.

cwgordon7’s picture

This looks good, can you please just roll both changes into one patch, made relative to the root Drupal directory? See http://drupal.org/patch/create if you need help creating patches. Once the patch is properly formatted, we can run it through our automated test system and make sure it passes tests. Thanks!

Bronsa’s picture

Status: Needs work » Needs review
FileSize
3.65 KB

Ok, here it is

webchick’s picture

Version: 7.x-dev » 8.x-dev
kscheirer’s picture

rerolled for HEAD.

RobLoach’s picture

FileSize
2.72 KB

YAML is what we're using for configs, but I have a hunch that we'll want this this compatibility layer for our .info files too. We might want to switch our .info files to YAML at some point, and this wrapper layer is the way to go for that.

Jeff Burnz’s picture

#19: 537332.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 537332.patch, failed testing.

Crell’s picture

Issue summary: View changes
Status: Needs work » Closed (won't fix)

I'm pretty sure we can close this issue, since we don't use info files anymore.