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

Automatic update the table of content if available #2869

Merged
merged 1 commit into from Apr 15, 2019

Conversation

dredav
Copy link
Contributor

@dredav dredav commented Feb 7, 2019

Description

After a line with a headline was changed
(the line should contains # ) in a note with a toc, the toc generator will generate a new toc for this note.

Tested use cases:

  • Edit a headline
  • Remove multiple lines with a headline
  • Paste lines with a headline
  • Undo / Redo

3b2hkavokh

Issue fixed

#2782

Type of changes

  • ⚪ Bug fix (Change that fixed an issue)
  • ⚪ Breaking change (Change that can cause existing functionality to change)
  • ⚪ Improvement (Change that improves the code. Maybe performance or development improvement)
  • 🔵 Feature (Change that adds new functionality)
  • ⚪ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

  • 🔵 My code follows the project code style
  • ⚪ I have written test for my code and it has been tested
  • 🔵 All existing tests have been passed
  • 🔵 I have attached a screenshot/video to visualize my change if possible

@Rokt33r Rokt33r added the awaiting review ❇️ Pull request is awaiting a review. label Feb 8, 2019
linePossibleContainsHeadline (currentLine) {
// We can't check if the line start with # because when some write text before
// the # we also need to update the toc
return currentLine.includes('# ')
Copy link
Member

Choose a reason for hiding this comment

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

Why can't we? We can just replace it with '\n# ' don't you think?

Copy link
Contributor Author

@dredav dredav Feb 11, 2019

Choose a reason for hiding this comment

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

I didn't think that matching '\n# ' helps. The line starts with the first char that is't a new line.
And we should check for the '# ' because when you add to a headline with '## test' something at the beginning (new text is: 'something## test') the toc should automatic remote the entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

You're right!

@ZeroX-DG ZeroX-DG added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Feb 9, 2019
Copy link
Member

@ZeroX-DG ZeroX-DG left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

linePossibleContainsHeadline (currentLine) {
// We can't check if the line start with # because when some write text before
// the # we also need to update the toc
return currentLine.includes('# ')
Copy link
Member

Choose a reason for hiding this comment

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

You're right!

@ZeroX-DG ZeroX-DG added approved 👍 Pull request has been approved by sufficient reviewers. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Feb 17, 2019
@Rokt33r Rokt33r added this to the v0.11.15 milestone Apr 15, 2019
@Rokt33r Rokt33r removed the approved 👍 Pull request has been approved by sufficient reviewers. label Apr 15, 2019
@Rokt33r Rokt33r merged commit 1675e04 into BoostIO:master Apr 15, 2019
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

3 participants