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: insert note index #undefined in case missing tags #2789

Merged
merged 3 commits into from
May 2, 2022
Merged

fix: insert note index #undefined in case missing tags #2789

merged 3 commits into from
May 2, 2022

Conversation

huland
Copy link
Contributor

@huland huland commented Apr 19, 2022

Community PR Review Checklist

Problem description

  • When insert note index command runs it collects the active note's child notes.
  • These notes will be checked with the condition which was modified.
  • The condition checked if the note's name startsWith "tags".
  • If it was true, the next line splits the note's name at "tags." and returns the 1st element (without checking the result of the split).

First Time Specifics

  • if its your first pull request to Dendron, watch out for the CLA bot that will ask you to agree to Dendron's CLA
  • if its your first pull request and you're on our Discord, make sure that Kevin gives you the horticulturalist role by adding your discord as part of the description πŸ‘¨β€πŸŒΎπŸ‘©β€πŸŒΎ
  • (optional) ping @Dendron Team in the #dev channel of our discord - we usually respond to PRs within 24h

Commit

Code

Tests

Docs

  • if your change reflects documentation changes, also submit a PR to dendron-site and mention the doc PR link in your current PR
  • does this change introduce a new or better way of doing things that others need to be aware of? if so, an async should be created and a process added in Development or Packages

@CLAassistant
Copy link

CLAassistant commented Apr 19, 2022

CLA assistant check
All committers have signed the CLA.

@kevinslin
Copy link
Member

kevinslin commented Apr 20, 2022

thanks for the PR! could you elaborate on the issue this is fixing? could you also add a test case in

?

@huland
Copy link
Contributor Author

huland commented Apr 21, 2022

I added:

  • description about the issue; see PR's "problem description" section
  • test cases

@huland huland requested a review from hikchoi April 30, 2022 21:25
Copy link
Contributor

@hikchoi hikchoi left a comment

Choose a reason for hiding this comment

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

test update looks good.
I approved the CI run and it looks good, but would be good to rebase to the latest master and check that the CI runs okay as well.

@huland
Copy link
Contributor Author

huland commented May 2, 2022

test update looks good. I approved the CI run and it looks good, but would be good to rebase to the latest master and check that the CI runs okay as well.

Thank you. I did rebase the latest master.

@kevinslin
Copy link
Member

this looks great! thanks for the contribution - we'll be including this in next weeks release!

@kevinslin kevinslin merged commit e025fd2 into dendronhq:master May 2, 2022
@huland huland deleted the fix/insert-note-index-undefined branch May 3, 2022 19:35
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

4 participants