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

Parse override tag arguments exactly like VSFilter #94

Closed
wants to merge 5 commits into from

Conversation

astiob
Copy link
Member

@astiob astiob commented May 13, 2014

This is the master issue for all your argument splitting bugs.

Creating this pull request just so we can have this master issue, even though _the code isn’t presentable._ And I mean it: just look at that faux C++ and Ruby. Do note and comment on my tag macro though; I think it’s pretty nice.

Although actually, isn’t it almost done? Maybe I’ll try to finish it quickly. Maybe. So I did.

Known bugs/missing things (check/tick means it’s fixed):

@astiob astiob self-assigned this May 13, 2014
This was referenced May 17, 2014
@astiob astiob changed the title [WIP] Parse override tag arguments exactly like VSFilter Parse override tag arguments exactly like VSFilter May 17, 2014
@astiob
Copy link
Member Author

astiob commented May 17, 2014

I’m sure trim_left and trim_right can and should be used in more places that are not currently touched by this PR. We can either add them all to the commit that introduces trim_left and trim_right or do that separately later.

@astiob
Copy link
Member Author

astiob commented May 17, 2014

Oh, and trim_right should be tested for speed.

@ghost
Copy link

ghost commented May 17, 2014

All in all, looks pretty ok IMHO.

@astiob
Copy link
Member Author

astiob commented May 17, 2014

I decided to just go with whatever we already use for whitespace, which happens to be just and\t. (Note that\rand\ndon’t matter.) So I modified my functions and looked around for other instances where they might be useful. Guess what… I discovered we hadskip_spacesandrskip_spaces in ass.c.

@astiob
Copy link
Member Author

astiob commented May 17, 2014

And now with whole-string drawing text append.

@grigorig
Copy link
Member

Looks good to me as well. I assume you have done a good amount of testing.

@astiob astiob closed this Jun 6, 2014
@astiob astiob deleted the argparse branch June 6, 2014 14:20
@astiob
Copy link
Member Author

astiob commented Jun 6, 2014

Merged. (Once again, I failed to remember to push the rebased branch to my fork first and then push it to master to automatically mark the pull request as merged.)

tgoyne pushed a commit to Aegisub/libass that referenced this pull request Nov 24, 2018
The code was confusing glyph index and Unicode codepoint.
256df61 attempted a fix but merely moved the wrong code.

Fixes Google Code issue libass#94.

Vertical ligatures are possibly still broken, but
horizontal ligatures and non-ligatures are correct now.
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

2 participants