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

Re-implementing Parser: Add support for multi-document tag directives #142

Merged
merged 2 commits into from Sep 18, 2015

Conversation

derFunk
Copy link
Contributor

@derFunk derFunk commented Sep 17, 2015

Especially to aid reading Unity scene files
Original PR #102

Andreas Katzig added 2 commits September 17, 2015 16:20
…tag directives - especially to aid reading Unity scene files
If the type could not be loaded, because it fails with an exception, make sure to return false to pass resolving to the next resolver in the list.
@derFunk
Copy link
Contributor Author

derFunk commented Sep 17, 2015

See also #140

@aaubry
Copy link
Owner

aaubry commented Sep 17, 2015

This looks good, thanks! There was the question of the different behavior in YAML 1.2, but I think we can leave that for later.

There is just an issue with the formatting. This project uses tabs instead of spaces, and your changes are using spaces, which messes-up the alignment of blocks. Could you update the PR with tabs instead of spaces?

PS: I know that most people prefer to use spaces instead of tabs. I have opened a separate issue, #143 to discuss a possible move to that convention. Feel free to leave a comment there.

@aaubry aaubry merged commit 0a877ea into aaubry:master Sep 18, 2015
@aaubry
Copy link
Owner

aaubry commented Sep 18, 2015

I made the changes myself because I had to resolve a merge conflict.

@derFunk
Copy link
Contributor Author

derFunk commented Sep 21, 2015

Ok, thanks a lot, I wasn't in reach of a PC this weekend :) 👍

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