{"action":"created","issue":{"url":"https://api.github.com/repos/composer/composer/issues/5364","repository_url":"https://api.github.com/repos/composer/composer","labels_url":"https://api.github.com/repos/composer/composer/issues/5364/labels{/name}","comments_url":"https://api.github.com/repos/composer/composer/issues/5364/comments","events_url":"https://api.github.com/repos/composer/composer/issues/5364/events","html_url":"https://github.com/composer/composer/issues/5364","id":156119286,"number":5364,"title":"`composer.json` does not allow comments","user":{"login":"cweagans","id":101590,"avatar_url":"https://avatars.githubusercontent.com/u/101590?v=3","gravatar_id":"","url":"https://api.github.com/users/cweagans","html_url":"https://github.com/cweagans","followers_url":"https://api.github.com/users/cweagans/followers","following_url":"https://api.github.com/users/cweagans/following{/other_user}","gists_url":"https://api.github.com/users/cweagans/gists{/gist_id}","starred_url":"https://api.github.com/users/cweagans/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cweagans/subscriptions","organizations_url":"https://api.github.com/users/cweagans/orgs","repos_url":"https://api.github.com/users/cweagans/repos","events_url":"https://api.github.com/users/cweagans/events{/privacy}","received_events_url":"https://api.github.com/users/cweagans/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":23,"created_at":"2016-05-21T20:07:06Z","updated_at":"2016-05-24T15:02:25Z","closed_at":"2016-05-23T07:46:29Z","body":"**I know lack of commenting is a limitation of the JSON format. I'm not attempting to convince anyone that YAML > JSON or that Composer should switch. Simply trying to get to a solution here.**\r\n\r\nAs developers, we all generally accept that commenting your code is a good thing. As developers, we would also not be too pleased if we had to write all of the code at the top of the file, and then have a specific section at the end of the file where all of the comments needed to live. That would significantly decrease the usefulness of the comments, and the comments would be much more likely to be outdated because they don't exist in the same context as the code that they're talking about.\r\n\r\nThe unfortunate reality is that by using JSON for the Composer file format, we're locked into that very situation. We have to create arbitrary keys in the `extra` section to document anything that's in `composer.json`.\r\n\r\nThis should not be the case.\r\n\r\nGiven that switching to YAML (or other formats) has been ruled out [over](https://github.com/composer/composer/issues/159) and [over](https://github.com/composer/composer/issues/440) and [over](https://github.com/composer/composer/issues/1778) and [over](https://github.com/composer/composer/issues/3228) again, and that I'm not really interested in opening that can of worms, we should try to find some other solution that allows Composer users to properly document things in their `composer.json`. Comments are not an uncommon need, especially for large projects with lots of dependencies.\r\n\r\nThere are a couple of solutions that I'd like to suggest (in order of my preference):\r\n\r\n1. Swap out the JSON parser for an [HJSON](http://hjson.org/) parser (PHP parser already written here: https://github.com/laktak/hjson-php, and it's backward compatible with plain JSON). This seems like a somewhat worthwhile compromise, as it doesn't require anyone to change anything with their projects, but allows people to start using a relaxed syntax + comments in their `composer.json`. Alternatively, upgrade to [JSON5](http://json5.org/) which supports comments, among other things. Looks like there's an existing package for this too: https://packagist.org/packages/hiroto-k/json5-php. Also backward compatible.\r\n\r\n2. Change `JsonFile.php` (and anywhere else that package data is read from `composer.json` -- ideally, this would all be consolidated appropriately) such that it strips any lines that begin with `//` (I think we could skip block comments for the sake of simplicity) before doing anything with the contents of the file. Note that Douglas Crockford suggested something similar: https://plus.google.com/+DouglasCrockfordEsq/posts/RK8qyGVaGSr. I'm not sure this is actually against the JSON spec, either, according to https://groups.yahoo.com/neo/groups/json/conversations/messages/152. I think this would require updates for Satis and Packagist as well, but they should be pretty similar changes to what would be required in Composer.\r\n\r\n3. Add a `_comment` property to the `composer.json` schema, ideally in a way that it can appear multiple times in a file without breaking validation. This has the benefit of `composer.json` still being valid JSON, and while it's a bit verbose, comments would be easier to deal with. Bonus points if `_comment` can appear as a subproperty of any other existing property. Also consider just using `_` or `#` or `//` as the key to cut down on extra typing to add a comment.\r\n\r\n4. If `composer.json` is not found but there's a `composer.yml` present, convert `composer.yml` -> `composer.json`, and then run composer as normal. Yes, this could be easily implemented as a wrapper around Composer, but I'd argue that sane commenting is a widely useful thing to have, and that it shouldn't be relegated to third party tooling. It should be a feature built directly into composer. Note that it's entirely reasonable (IMO) for Packagist and the like to not change their behaviors -- that is, the generated `composer.json` would still need to be committed to the repository (probably in addition to the `composer.yml`). This obviously satisfies the requirement of being able to add comments, but adding an additional translation step is not ideal. Note that this doesn't necessarily need to be YAML - this could be something as simple as `composer.cjson` (commented JSON) -> `composer.json` (normal `composer.json` that we have today).\r\n\r\nAt the end of the day, I don't really care how this gets solved as long as I can put comments in my `composer.json`. If we come to an agreement on the direction to take, I'm willing and able to implement it. I didn't want to spend a bunch of time on something that would be rejected though, so I thought I'd open this issue first to discuss whether or not any of the ideas above would be accepted.\r\n\r\nThoughts?"},"comment":{"url":"https://api.github.com/repos/composer/composer/issues/comments/221299984","html_url":"https://github.com/composer/composer/issues/5364#issuecomment-221299984","issue_url":"https://api.github.com/repos/composer/composer/issues/5364","id":221299984,"user":{"login":"cweagans","id":101590,"avatar_url":"https://avatars.githubusercontent.com/u/101590?v=3","gravatar_id":"","url":"https://api.github.com/users/cweagans","html_url":"https://github.com/cweagans","followers_url":"https://api.github.com/users/cweagans/followers","following_url":"https://api.github.com/users/cweagans/following{/other_user}","gists_url":"https://api.github.com/users/cweagans/gists{/gist_id}","starred_url":"https://api.github.com/users/cweagans/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cweagans/subscriptions","organizations_url":"https://api.github.com/users/cweagans/orgs","repos_url":"https://api.github.com/users/cweagans/repos","events_url":"https://api.github.com/users/cweagans/events{/privacy}","received_events_url":"https://api.github.com/users/cweagans/received_events","type":"User","site_admin":false},"created_at":"2016-05-24T15:02:25Z","updated_at":"2016-05-24T15:02:25Z","body":"> The MakeUpYourMindException would break thousands of existing CI/CD systems out there not cleaning up their folders and suddenly seeing both files end up simultaneously.\r\n\r\nWhat? That's not true at all. If I have a project with `composer.json` in it and I upgrade to a version of Composer that has this new functionality, nothing changes. If I then *add* a `composer.hjson` (or whatever) without removing `composer.json`, then I get the exception. It takes two separate conscious actions to get to that point. It should not be a \"suddenly everything is broken\" kind of thing at all.\r\n\r\n> What is wrong with my solution? Or the one @Seldaek suggested (using _ as a key)? It's backwards compatible, doesn't require any changes, and makes all parties involved in this discussion happy.\r\n\r\nI guess it works, but it's very verbose and pretty suboptimal, but if that's the only thing we're going to be able to do, it's better than nothing, I suppose.\r\n\r\n> But you want it to be the Composer team decision, seriously?\r\n\r\nYou know what I mean. It shouldn't live outside of Composer.\r\n\r\n> But actual supported version by Composer is \"php\": \"^5.3.2 || ^7.0\". Why didn't you check it out before offering?! [...]\r\n> Why composer.json does not support multiline? I want to write detailed description for my super-duper awesome package.\r\n> Why composer.json does not support inheritance/reference? I want to reuse some code and adhere to DRY principle.\r\n\r\nSo? \"You have to upgrade PHP to use the HJSON format in Composer.\" Done. Or we could consider a different tool that's arguably better for the job, but I've been purposely trying to not steer the discussion there (hint: it rhymes with BLAML).\r\n\r\nIn general, a lot of your arguments come down to \"What if ____ happens or user wants _____?\". Taking a feature request to it's most ridiculous extreme to disprove the need for the original feature request is kind of disingenuous, if you ask me. To the users that want imports or multiline or whatever, a \"no, do it elsewhere\" is acceptable. But the fact that there have been so many issues opened about wanting simple comments in a file that *can get really damn complicated for large projects* is telling. There is a need for this functionality.\r\n\r\n> Don't do Package Management Driven Development! It is not important in most cases! If you really need it you get it by yourself (start using #5364 (comment)). I'm done and out from this discussion.\r\n\r\nWhat? We're not talking about an entire new development methodology here, man. We're talking about adding *comments*. This should not be rocket surgery.\r\n\r\n> Fixed the _comment validation in 98b0af1 which makes official that \"workaround\".\r\n\r\nThanks, @Seldaek. I appreciate that. Taking this a step further, If I were to implement something that satisfies the following criteria, would you be willing to consider it? And are there any criteria I'm missing?\r\n\r\n* Adds support for a JSON superset or YAML\r\n* 100% backward compatible and opt-in only\r\n* Supported by Composer, Satis, and Packagist\r\n* Doesn't unacceptably bloat Composer\r\n* Doesn't change Composer's version constraints to anything more restrictive\r\n\r\n"}}
{"actor":{"display_login":"cweagans"}}