Skip to content
This repository has been archived by the owner on Mar 8, 2019. It is now read-only.

Resolved issue #4 Support bullet lists starting with * or + #18

Merged
merged 8 commits into from
Feb 4, 2016

Conversation

maodd
Copy link

@maodd maodd commented Feb 1, 2016

Nothing special, original state was - syntax only. I added * and + in grammar file, re-run generate command to update token file. Only issue was * conflicting with em1 syntax (** **), trick is to pull list parser before em.

NSParagraphStyleAttributeName: paragraphStyle};
label.attributedText = [parser attributedStringFromMarkdownString:markdown];

FBSnapshotVerifyView(label, nil);
Copy link
Contributor

Choose a reason for hiding this comment

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

why have you replaced this test?

@gskbyte
Copy link
Contributor

gskbyte commented Feb 2, 2016

@maodd thanks for your pull request! However, tests are still red, could you please fix them in order to merge your changes? Thanks!

@gskbyte
Copy link
Contributor

gskbyte commented Feb 2, 2016

@maodd it looks the problem is that you are running tests against iOS 9 and the CI runs them on iOS 8.1. Could you please replace it in the Rakefile? Travis should be able to run the tests on iOS 9.3: https://docs.travis-ci.com/user/languages/objective-c/

@maodd
Copy link
Author

maodd commented Feb 2, 2016

I re did record in iOS 8.1 instead. but seems still couldn't make test happy.

one of the failed test is testHeaders:

screen shot 2016-02-02 at 9 30 43 am
screen shot 2016-02-02 at 9 30 54 am

I just can't tell the difference between those 2 images.

@maodd
Copy link
Author

maodd commented Feb 2, 2016

test plaintext result: snapshot image area look different. why?

screen shot 2016-02-02 at 9 40 08 am
screen shot 2016-02-02 at 9 40 16 am

@maodd
Copy link
Author

maodd commented Feb 2, 2016

Really can't tell the difference in testStyles result:

screen shot 2016-02-02 at 9 41 47 am
screen shot 2016-02-02 at 9 41 55 am

@maodd
Copy link
Author

maodd commented Feb 2, 2016

Font Change results compare:

screen shot 2016-02-02 at 9 43 35 am
screen shot 2016-02-02 at 9 43 42 am

@maodd
Copy link
Author

maodd commented Feb 2, 2016

test paragraph result compare:

screen shot 2016-02-02 at 9 44 42 am
screen shot 2016-02-02 at 9 44 48 am

@maodd
Copy link
Author

maodd commented Feb 2, 2016

Good news is 3 of 8 tests passed (including my newly added testLists)
Bad news is the other 5 failed and my eyes can't tell the difference but Kaleido can.

@maodd
Copy link
Author

maodd commented Feb 2, 2016

OK. all tests passed now. need to record in iPhone5s.

case MARKDOWN_NEWLINE: {
textAsString = @"";
break;
}
case MARKDOWN_URL: {

if ([textAsString hasPrefix:@"@"]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

Copy link
Author

Choose a reason for hiding this comment

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

this for another issue (#17) reported and I also encountered, domain in email address shouldn't not be rendered as link. I tried to do pure regex fix, no good. ended up with let regex expend optional leading at sign (@) , and let code to skip domain in email.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, that might be valid as a temporal fix

@gskbyte
Copy link
Contributor

gskbyte commented Feb 3, 2016

Thanks for fixing the tests @maodd ! I just left a small comment on your code, after you answer we can merge the PR :)

gskbyte added a commit that referenced this pull request Feb 4, 2016
@gskbyte gskbyte merged commit b04ab3e into xing:master Feb 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants