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

rewrite zephyr scribelike markup parsing #172

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

kcr
Copy link
Member

@kcr kcr commented Mar 15, 2016

While investigating something that turned out to be trac ticket #188, I looked at owl_fmtext_append_ztext and found that I couldn't unsee it, so here's a parser with a state machine that produces a syntax tree, that then gets rendered into fmtext. This turned out not to fix the bug; herein also find a ztext_protect function that can be used to fix the bug. Also a zwgc-style code strip (that doesn't just strip environments that barnowl understands) and support for @ roman (remove bold and underline).

@@ -31,6 +31,7 @@ The following people have provided patches or other contributions:
DD Liu
Betsy Riley
Robert Jacobs
Google Inc.
Copy link
Member

Choose a reason for hiding this comment

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

Is Google Inc. actually a person who contributed?

Copy link
Member Author

Choose a reason for hiding this comment

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

They claim copyright on the code in the patch, and the AUTHORS file is, among other things, the list of copyright holders.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps s/people/entities/ on line 4?

"Corporations are people you know" :-}

@andersk
Copy link
Member

andersk commented Mar 21, 2016

This code is on the critical path for search, so I worry a little about the performance implications of replacing a strpbrk-based parser with something that does table lookups and opcode interpretation on every character. Have you benchmarked it?

@kcr
Copy link
Member Author

kcr commented Mar 22, 2016

I have not, because it hadn't occurred to me, at least partially because I can reason about my code's time complexity (it was written to be O(n) time on the size of the input) and I can't duplicate the reasoning about the original code.

(Also strpbrk is doing the same sort of table lookup (albeit in assembly language) and bulding the (admittedly simpler) table on every call.)

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