Skip to content

Conversation

@JinneeJ
Copy link
Contributor

@JinneeJ JinneeJ commented Aug 3, 2016

Finally added ability to get clean text.

P.S.: Really sorry for big delay 🙏


@Override
public void visit(OrderedList orderedList) {
orderedListCounter = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably use orderedList.getStartNumber() instead of 1, right?

Copy link
Contributor Author

@JinneeJ JinneeJ Sep 14, 2016

Choose a reason for hiding this comment

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

You're right :) Fixed here 32771fa

@robinst
Copy link
Collaborator

robinst commented Aug 30, 2016

Nice change!

Do you have any plans for making it work for custom nodes for extensions (e.g. Strikethrough)? Maybe we should have a similar model as for HTML rendering where extensions can contribute code for rendering, and users can customize the rendering for specific node types. See NodeRenderer and friends.

(By the way, I was on holidays, that's why it took so long to respond.)

@JinneeJ
Copy link
Contributor Author

JinneeJ commented Sep 14, 2016

Thanks for review! I've made the fix with lists.

Also I've tried to implement support extensions for TextContentRenderer. But I had to make some changes in core in order to avoid copying of code. You can see it in last 5 commits. Let me know what you think about those changes 😊

@robinst
Copy link
Collaborator

robinst commented Sep 20, 2016

Cool, starting to look good!

I wanted to make a few suggestions but wasn't sure how they'd work, so I checked out the branch and started modifying it :). I merged master into your branch and did these changes on top:

  1. 43c7e28
  2. 91827e2

Can you have a look and tell me if it makes sense?

@JinneeJ
Copy link
Contributor Author

JinneeJ commented Sep 22, 2016

I think your changes are great 😊

@robinst robinst merged commit bb48e51 into commonmark:master Sep 23, 2016
@robinst
Copy link
Collaborator

robinst commented Sep 23, 2016

Thanks! Merged now. Will do a release soon with these and other changes.

@robinst
Copy link
Collaborator

robinst commented Sep 23, 2016

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