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.

Sign up for GitHub

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

Jump to bottom

Automatic table of contents generation for Markdown #2281

Merged
merged 7 commits into from
Sep 15, 2018
Merged

Automatic table of contents generation for Markdown #2281

merged 7 commits into from
Sep 15, 2018

Conversation

mbarczak
Copy link

@mbarczak mbarczak commented Aug 10, 2018 •
edited
Loading

This PR aims to solve #1436.

  • Adds table of contents for any currently displayed Markdown note or Markdown snippet, at current cursor position.
  • Consequent generations update existing TOC.
  • Generated TOC is case sensitive to handle Table of Contents Links are Case Sensitive #2067

Shortcut : SHIFT+CTRL+T
Menu : File / Generate/Update Markdown TOC

Automatic table of contents generation for Markdown
7804a22 
Adds table of contents for any Markdown note or
Markdown snippet.
Consequent generations update existing TOC.
Generated TOC is case sensitive to handle BoostIO#2067

Shortcut : CommandOrControl+Alt+T
Menu : Edit/Generate/Update Markdown TOC
@IssueHuntBot IssueHuntBot mentioned this pull request Aug 10, 2018
Closed
@mbarczak mbarczak closed this Aug 11, 2018
@mbarczak mbarczak reopened this Aug 11, 2018
@kazup01 kazup01 added the awaiting review ❇️ Pull request is awaiting a review. label Aug 13, 2018
Change menu position and accelerator for TOC gen.
ce3b290 
Due to the fact, that submenu "Edit" is visible only in macOS,
let's move TOC generator to "File" menu. Also, change
accelerator to SHIFT+CTRL+T which is working without conflicts
and problems on all platforms.
@Rokt33r Rokt33r 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 Aug 20, 2018
Rokt33r
Rokt33r requested changes Aug 20, 2018
View reviewed changes
Copy link
Member

@Rokt33r Rokt33r left a comment

Choose a reason for hiding this comment

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

Could you make a test for this function?

https://github.com/BoostIO/Boostnote/pull/2281/files#diff-6a71191a63c2cc8d596fd5acceb9e08cR41

mbarczak
mbarczak commented Aug 20, 2018
View reviewed changes
Copy link
Author

@mbarczak mbarczak left a comment

Choose a reason for hiding this comment

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

@Rokt33r , good point. I've added unit test with 13 test cases.

@ZeroX-DG
Copy link
Member

LGTM 🎉

ZeroX-DG
ZeroX-DG reviewed Aug 25, 2018
View reviewed changes
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.

I think this need some small changes before it get merged.

browser/main/Detail/MarkdownNoteDetail.js Outdated Show resolved Hide resolved
browser/main/Detail/SnippetNoteDetail.js Outdated Show resolved Hide resolved
@ZeroX-DG
Copy link
Member

Hey @mbarczak , just one more small request, it would be great if you can insert the toc at the cursor position. Sometimes, I want to have a document like this:

This is some introduction...
<!--TOC-->
# h1
## h2

With the current code it only insert at the top of the document.

Create Markdown TOC at current cursor position
1bb841d 
If there is no TOC in the current document, it's created at current
cursor position. Subsequent generation calls update TOC at existing
position.

Add additional tests with CodeMirror editor mock.
@mbarczak
Copy link
Author

@ZeroX-DG , I've introduced change you requested, and indeed, it's more usable now. Also, I've added additional tests with CodeMirror mock.

ZeroX-DG
ZeroX-DG requested changes Sep 2, 2018
View reviewed changes
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.

Just another small request 😄 can you please change your code. Thank you!

browser/main/Detail/SnippetNoteDetail.js Outdated Show resolved Hide resolved
browser/main/Detail/SnippetNoteDetail.js Outdated Show resolved Hide resolved
@mbarczak
Copy link
Author

mbarczak commented Sep 4, 2018

@ZeroX-DG , all requests solved ;)

ZeroX-DG
ZeroX-DG approved these changes Sep 5, 2018
View reviewed changes
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 🎉 Thank you for your contribution!

@ZeroX-DG ZeroX-DG added next release (v0.11.10) and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Sep 5, 2018
@mbarczak
Copy link
Author

mbarczak commented Sep 6, 2018

Hi @Rokt33r , work is done here. Ready to be merged.

@Rokt33r Rokt33r merged commit 7ab3ce9 into BoostIO:master Sep 15, 2018
This was referenced Oct 9, 2018
Closed
Closed
@kazup01
Copy link
Member

kazup01 commented Oct 21, 2018

Hi @mbarczak, I featured your contribution on release blog 😄 Thank you!
https://medium.com/boostnote/boostnote-v0-11-10-release-on-halloween-9bbe6d488fe8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rokt33r Rokt33r Rokt33r requested changes

@ZeroX-DG ZeroX-DG ZeroX-DG approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mbarczak @ZeroX-DG @kazup01 @Rokt33r