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

Advanced YAML component usage #6582

Merged
merged 1 commit into from
May 21, 2016
Merged

Advanced YAML component usage #6582

merged 1 commit into from
May 21, 2016

Conversation

dantleech
Copy link
Contributor

@dantleech dantleech commented May 21, 2016

Q A
Doc fix? no
New docs? yes
Applies to 2.3 > 3.0 (i think)
Fixed tickets #6524

Prerequisite to #6226

This PR documents the (now deprecated) features for indentation, invalid types and object serialization which have been present since sf 2.3

@HeahDude
Copy link
Contributor

Thanks, this closes #6524.

$dumper->dump($data, 2, 4, true);

// throw an exception if an encoded object is found in the YAML string
$parser->parse($yaml, true); // throw an exception
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove duplicated throw an exceptiong

@dantleech
Copy link
Contributor Author

dantleech commented May 21, 2016

Missing documentation for Parse(.. $references) not part of the pulic API.

@dantleech
Copy link
Contributor Author

Also 2.5 introduced the $objectForMap argument to parse

By default the YAML component will use 4 spaces for indentation, this can be
changed using the second argument as follows:

.. code-block:: php
Copy link
Member

Choose a reason for hiding this comment

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

please use the :: shortcut here (meaning the colon on line 229 should be changed to a double one and line 230 and 231 should be removed)

@wouterj
Copy link
Member

wouterj commented May 21, 2016

I like this one, it's straight to the point and clear. Thanks!

@dantleech
Copy link
Contributor Author

Updated. As noted above 2.5 introduced a new argument. Shall I wait until this is resolved / merged before making a PR on 2.5, and then finally on 3.1 ?

$yaml = Yaml::dump($array);

Array Expansion and Inlining
""""""""""""""""""""""""""""
Copy link
Member

Choose a reason for hiding this comment

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

We usually use dots (.) for the 4th headline level.

@HeahDude
Copy link
Contributor

HeahDude commented May 21, 2016

@dantleech 2.5 is not maintained anymore, so I suggest you wait for this to be merged on 2.7 to send another PR.

You will need to add a note version added where you can point 2.5.

Indentation
"""""""""""

By default the YAML component will use 4 spaces for indentation, this can be
Copy link
Member

Choose a reason for hiding this comment

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

I would split this in two sentences: "[...] for indentation. This can be [...]"

@wouterj
Copy link
Member

wouterj commented May 21, 2016

@dantleech you can already create a PR for 2.7 and then rebase after this PR is merged. That allows us to review things already so merging will be a quick process.

@dantleech
Copy link
Contributor Author

Updated.

type is encountered in either the dumper or parser as follows::

// throw an exception if a resource or object is encoutered
$dumper->dump($data, 2, 4, true);
Copy link
Member

Choose a reason for hiding this comment

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

Yaml:dump(...)

Copy link
Contributor Author

@dantleech dantleech May 21, 2016

Choose a reason for hiding this comment

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

The reason I used the dumper is because it was already used in the Writing YAML files section more than once. Assuming now that after we declare the shortcut we should continue using the shortcut.

@xabbuh
Copy link
Member

xabbuh commented May 21, 2016

Just left some last minor comments, but overall this looks quite great!

@dantleech dantleech force-pushed the advanced_yaml branch 2 times, most recently from 8917c91 to f192d5b Compare May 21, 2016 12:51
@dantleech
Copy link
Contributor Author

Updated

@xabbuh
Copy link
Member

xabbuh commented May 21, 2016

I just noticed that the chapter contains this sentence:

Of course, the Symfony Yaml dumper is not able to dump resources. Also, even if the dumper is able to dump PHP objects, it is considered to be a not supported feature.

What do we do with it? Remove it?

Instead of encoding as ``null`` you can choose to throw an exception if an invalid
type is encountered in either the dumper or parser as follows::

// throw an exception if a resource or object is encoutered
Copy link
Member

Choose a reason for hiding this comment

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

encountered

@dantleech
Copy link
Contributor Author

What do we do with it? Remove it?

Hmm. Either we remove that or the new documentation, or we add a warning.

@xabbuh
Copy link
Member

xabbuh commented May 21, 2016

My suggestion: Let's remove the existing one and add a warning in the new section telling people that object support is a feature only existing in the Symfony Yaml component and thus (as not being part of the standard) very likely not supported in other implementations.

@javiereguiluz
Copy link
Member

@xabbuh I'd say remove it. The code display show a clear warning when trying to dump resources, so the docs can safely ignore this edge case.

@dantleech
Copy link
Contributor Author

dantleech commented May 21, 2016

Removed the previous note:: and added a warning:: ..

@xabbuh
Copy link
Member

xabbuh commented May 21, 2016

👍

...........

By default the YAML component will use 4 spaces for indentation. This can be
changed using the second argument as follows::
Copy link
Member

Choose a reason for hiding this comment

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

the second argument -> the third argument ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, yes.


.. warning::

Object seialization is specific to this implementation, other PHP YAML
Copy link
Member

Choose a reason for hiding this comment

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

typo: "serialization"

@wouterj wouterj merged commit 3249cbb into symfony:2.3 May 21, 2016
wouterj added a commit that referenced this pull request May 21, 2016
This PR was merged into the 2.3 branch.

Discussion
----------

Advanced YAML component usage

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes
| Applies to    | 2.3 > 3.0 (i think)
| Fixed tickets | #6524

Prerequisite to #6226

This PR documents the (now deprecated) features for indentation, invalid types and object serialization which have been present since sf 2.3

Commits
-------

3249cbb Advanced YAML component usage
wouterj added a commit that referenced this pull request May 21, 2016
@wouterj
Copy link
Member

wouterj commented May 21, 2016

Thanks @dantleech! This is now merged into all branches of the docs, so you can start working on the dumper flags if yo uwant.

xabbuh added a commit that referenced this pull request May 21, 2016
This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #6590).

Discussion
----------

Added note on YAML mappings as objects

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes
| Applies to    | 2.7 > 3.0

Depends on #6582

Commits
-------

e9de4ca Added note on YAML mappings as objects
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.

None yet

5 participants