Skip to content

Conversation

leebyron
Copy link
Contributor

A couple issues have opened up about inconsistency in rendering graphql queries in the spec and in the ref impl - especially with respect to whitespace and commas.

Here I'm proposing that it's easier to read queries where every field is always on it's own line (eg, no image { width, height } one-liners) and that lines are split only by line-terminators and not also commas.

If there's agreement around this proposal, the second step will be to apply the printer to all graphql examples in the spec.

A couple issues have opened up about inconsistency in rendering graphql queries in the spec and in the ref impl - especially with respect to whitespace and commas.

Here I'm proposing that it's easier to read queries where every field is always on it's own line (eg, no `image { width, height }` one-liners) and that lines are split only by line-terminators and not also commas.

If there's agreement around this proposal, the second step will be to apply the printer to all graphql examples in the spec.
@schrockn-zz
Copy link
Contributor

Makes sense to me.

My only request would be a "russHanneman" flag where the delimiter is tres comas (",,,") rather than a single comma ","

@schrockn-zz
Copy link
Contributor

We also need to do a pass in the spec to make all examples conform to that

@leebyron
Copy link
Contributor Author

,,,

@dschafer
Copy link
Contributor

Yeah, agreed; I think this is better.

I do like the one-liners in some cases, but I don't think the standard printer should use them by any means.

@leebyron
Copy link
Contributor Author

I'm waiting on spec cleanup to make sure everyone's cool with the format :)

@leebyron
Copy link
Contributor Author

@dschafer I considered making the printer smarter but couldn't think about simple enough rules of when inlining is okay.

Within the spec I'll go for fairly aggressive consistency, elsewhere I agree - inlining is terse and nice

@schrockn-zz
Copy link
Contributor

You may merge at will.

@leebyron
Copy link
Contributor Author

Firing away, captain

leebyron added a commit that referenced this pull request Jul 15, 2015
[RFC] standard printer uses no comma between fields
@leebyron leebyron merged commit 7a3d7ab into master Jul 15, 2015
@leebyron leebyron deleted the printer branch July 15, 2015 01:31
@leebyron
Copy link
Contributor Author

Follow up on the spec can be found here graphql/graphql-spec@188cb06

otech47 added a commit to otech47/graphql-js that referenced this pull request Mar 21, 2016
adding an operation name of 'mutation' for a mutation query in a post request
otech47 added a commit to otech47/graphql-js that referenced this pull request Mar 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants