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 custom formatting to cli tessellate cmd #362

Merged
merged 1 commit into from
Jun 13, 2018

Conversation

zen3ger
Copy link

@zen3ger zen3ger commented Jun 3, 2018

What's working:

Same output as before using the default format string.
Custom format strings which don't contain {} inside expansion blocks (so far) work as expected.
Escaping {} in format string with \{ and \}.

For the future:

  • reduce string copies
  • simplify syntax

@zen3ger
Copy link
Author

zen3ger commented Jun 3, 2018

I think it worth mentioning that this can be finally done:

$ cargo run -- tessellate --format "@indices{sep=-}{Na} BATMAN!" "$TEST"
Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na-Na BATMAN!

@zen3ger
Copy link
Author

zen3ger commented Jun 5, 2018

@nical if you have time give it a try, you might end up writing some formatter that will fail. Also do you think the doc is enough?

cli/README.md Outdated
```
vertices: [@vertices{sep=, }{({position.x}, {position.y})}]\nindices: [@indices{sep=, }{{index}}]
```
If you want to use `{` and `}` in the format string you have to write `\{` and `\}`, as normally `{}` groups mark a variable expension. Here's an example:
Copy link
Author

Choose a reason for hiding this comment

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

typo, "expansion"

@nical
Copy link
Owner

nical commented Jun 5, 2018

I love it! I'm almost able to do a json output ( Running into issues with \}). It would be great to be able to do a bunch of other simple formats like .obj (for this one we might need to add a @triangles{sep}{formatter} that gives access to triplets of indices).

I'm happy to land this as is and keep iterating on it unless you prefer to do some more polish before it host the master branch.

@zen3ger
Copy link
Author

zen3ger commented Jun 6, 2018

I'll add at least @triangles before merge.

@zen3ger
Copy link
Author

zen3ger commented Jun 6, 2018

Ok, I added @triangles and found a way to do JSON like output without overcomplicating the regex patterns.
If the end of the iterator expander also marked with @ than any trailing \} would be parsed in a separate block. I decided to add {fmt=...} for the item format string just to make it consistent with the separator syntax.
The syntax became @indices{sep=, }{fmt={index}}@...

A JSON like output with @triangles:

$> lyon tessellate --format '\{\n "triangles": \{\n@triangles{sep=,\n}{fmt=  "triangle": \{\n   "{0}",\n   "{1}",\n   "{2}"\n  \}}@\n \}\n\}' "M 0 0 L 10 0  10 10 L 0 10 z"
{
 "triangles": {
  "triangle": {
   "1",
   "0",
   "2"
  },
  "triangle": {
   "1",
   "2",
   "3"
  }
 }
}

@zen3ger zen3ger mentioned this pull request Jun 6, 2018
3 tasks
@nical
Copy link
Owner

nical commented Jun 7, 2018

Nice! Why is the trailing @ needed if non-special } are also escaped?

@zen3ger
Copy link
Author

zen3ger commented Jun 12, 2018

\} and \{ was "escaped" via regex pattern. The \ is then removed before print. Unfortunately I couldn't figure out how not to match on \} chars after the {formater} block, ie. \{@indices{sep=,}{\{{index}\}}\} block, as the formater capture \{(.*?)\} would match until the first variable closing (\{{index and then }\}}\} is the left over string) and \{(.*)\} would match until the very last }.

Basically, @ used to split the formater into substrings for parsing so @indices{}{} @vertices{}{} is ["indices{}{}", " ","vertices{}{}"] and than matchin is done... By adding @ after the formatter any trailing \} would be in a different string which would be just pushed to the buffer without var expansion. Leading \{ are solved also via @ before the formater, and I fixed the varibale match regex to capture \{ before the var pattern.

If the @ after the formatter is anoying I'll come up with something better.

@nical
Copy link
Owner

nical commented Jun 13, 2018

I see. It'd be nice to come back and simplify the syntax eventually, but the feature is already super cool in its current state so I'm happy to merge this as is (I'd merge it right now but there is still [wip] in the title so I am not sure whether you want to add more stuff).

Add --format flag to tessellate subcommand.
Update CLI readme.

Closes nical#289
@zen3ger
Copy link
Author

zen3ger commented Jun 13, 2018

I updated the README and squashed the commits. I thinks it's ok to merge (for) now.

@zen3ger zen3ger changed the title [WIP] Add custom formatting to cli tessellate cmd Add custom formatting to cli tessellate cmd Jun 13, 2018
@nical nical merged commit 7b1f2fc into nical:master Jun 13, 2018
@zen3ger zen3ger deleted the cli-tessellate-formatter branch June 13, 2018 20:27
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.

2 participants