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

Fix issue67 #68

Merged
merged 2 commits into from
Jan 10, 2016
Merged

Fix issue67 #68

merged 2 commits into from
Jan 10, 2016

Conversation

syohex
Copy link
Collaborator

@syohex syohex commented Jan 10, 2016

Highlight italic only following cases

  • Italic start and end point are same line
  • If italic start and end point are different line, both lines are not list

Original

orig

Fixed version

fixed

This is related to #67.
CC: @hmelman

- Remove expected-result attribute
- Add tests start or end line is list
Highlight only following cases
- Italic start and end point are same line
- If italic start and end point are different line, both lines are not list
@jrblevin
Copy link
Owner

Thanks! I'll merge this, but I think there are some cases that might still slip through. For example, what about this:

* something about function
  foo_bar
* something else about
  foo_bar

I think checking all lines between begin and end for list markers will cover this case as well. Given that, might a regexp search for the list item regexp in the potential italic region be more efficient?

@jrblevin
Copy link
Owner

Also, regarding the tests you added, with this one Markdown.pl does generate a list item with italics:

* foo_bar
foo_bar

Now I'm thinking that checking for markdown-list-face in the possible italic range is a safer approach. It also covers the hanging list example above and is more efficient since we've already done the hard work of parsing the nested lists.

@jrblevin jrblevin merged commit f72ebac into jrblevin:master Jan 10, 2016
@jrblevin
Copy link
Owner

I updated this by adding a new test in 6aafece and changing to check for markdown-list-face in 84d94aa. Let me know if you see any problems.

@syohex syohex deleted the issue-67 branch January 10, 2016 23:05
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.

None yet

2 participants