Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to condense output in flow sequences #358

Merged
merged 4 commits into from Jul 7, 2017

Conversation

eseliger
Copy link
Contributor

@eseliger eseliger commented Jul 5, 2017

As described in #346, this is useful for using yaml as query parameters in URLs, for example.

Closes #346

@puzrin
Copy link
Member

puzrin commented Jul 5, 2017

Ok in general, but condense option name seems not enougth clear. Any ideas how to improve?

@puzrin
Copy link
Member

puzrin commented Jul 5, 2017

condensedFlow (not sure)?

@eseliger
Copy link
Contributor Author

eseliger commented Jul 5, 2017

Already had that, too, thought it was too long. But you're right, condense is not very clear.
If you agree, I could change it to condenseFlow. I think the name is ok, it does exactly that, condensing flow sequences, what do you think?

README.md Outdated
@@ -182,6 +182,7 @@ options:
- `noRefs` _(default: `false`)_ - if `true`, don't convert duplicate objects into references
- `noCompatMode` _(default: `false`)_ - if `true` don't try to be compatible with older
yaml versions. Currently: don't quote "yes", "no" and so on, as required for YAML 1.1
- `condense` _(default: `false`)_ - if `true` flow mode sequences will be condensed, omitting the space between `key: value` or `a, b`. Eg. `'[a,b]'` or `{a:{b:c}}`.
Copy link
Member

@puzrin puzrin Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear for newbies what is this option for (in real world). May be, add something "may be useful to create compact URL-s"?

@puzrin
Copy link
Member

puzrin commented Jul 5, 2017

I have no personal preferences and english is not my native language. So, i'd suggest you to take timeout for 1 day, may be you will get better ideas. This PR will be accepted, but i'd like it to be ideal if possible :)

@eseliger
Copy link
Contributor Author

eseliger commented Jul 5, 2017

I'll sleep one night over it, leaving you a comment tomorrow with my thoughts on this :)

@eseliger
Copy link
Contributor Author

eseliger commented Jul 7, 2017

Hey, thought about it.

In my opinion, condenseFlow describes pretty concise, what this flag does. The only thing this flag changes is omitting the spaces between array entries and and key, value pairs in objects, when in flow mode. So, in fact, it does condense the output to a minimum, where either arrays or objects are flowing. In conclusion, I think the combination of condense and flow to condenseFlow is probably the most fitting description for this flag. It condenses items in flow mode.

Your thoughts on that? :-)

@puzrin puzrin merged commit bd32506 into nodeca:master Jul 7, 2017
@eseliger eseliger deleted the condense branch July 7, 2017 23:14
@eseliger
Copy link
Contributor Author

eseliger commented Jul 7, 2017

Great! Could you release this change on npm as well? :)
That would be awesome!

@puzrin
Copy link
Member

puzrin commented Jul 7, 2017

On weekend ok? Need to rework pending PR

@eseliger
Copy link
Contributor Author

eseliger commented Jul 7, 2017

No problem, take the time you need!

@puzrin
Copy link
Member

puzrin commented Jul 8, 2017

published 3.9.0

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

Successfully merging this pull request may close these issues.

None yet

2 participants