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

YAMLParser: some small fixes #50

Closed
wants to merge 4 commits into from
Closed

YAMLParser: some small fixes #50

wants to merge 4 commits into from

Conversation

YaseenAlk
Copy link

@YaseenAlk YaseenAlk commented Aug 1, 2018

This consists of 4 commits to YAMLParser:

  • "Add .vs/ and YAMLParser sln files to the gitignore"

I used VS Code to open the repo, and I opened YAMLParser with Visual Studio. Updated the gitignore to ignore the config files for these 2 situations.

  • "Use System.IO.Path/Directory/DirectoryInfo for file/directory handling"

This commit does not (or rather, should not) change any part of YAMLParser's functionality. It was just to prep the project so that I could run YAMLParser on Mac/Linux using .NET Core CLI. I'm almost done making the necessary conversions, and might make a separate pull request in case that also sounds like a useful addition.

  • "Fix GUTS' character escaping for string constants"

Interestingly, I ran into an issue in YAMLParser where the following lines in a .msg file were not being parsed correctly:

string CONTENT_JSON = "application/json"
string CONTENT_STREAM = "application/octet-stream"

This commit (hopefully) fixes any issues with string constants.

  • "Ensure that std_msgs was processed before attempting to generate files"

While testing the .NET Core version of YAMLParser that I've been editing, I ran into an elusive problem where GenerateFiles() in Program.cs was stuck in an infinite loop while trying to MD5 hash some .msg files. The issue turned out to be in PrepareToHash(): within the method, irm.Stuff[i].Definer was always null, because the program could not resolve the SingleTypes of the messages without std_msgs first being processed. There may be a smarter way to fix this issue, but the easiest solution is to just avoid generating any files if std_msgs was not an included directory.

@YaseenAlk
Copy link
Author

I just noticed the fork by Xamla, and I now realize that this PR might just be reinventing the wheel... Feel free to implement any commits if you find them useful.

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.

1 participant