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: creating scratch when text is selected within a note SHOULD not match scratches just due to prefix #1292

Merged
merged 2 commits into from
Sep 7, 2021

Conversation

nickolay-kondratyev
Copy link
Contributor

Summary

Fix scratch creation when text is selected within a note.

Description

  1. Write out a new string

some new string that you want to add to scratch

  1. Use the create scratch note shortcut on the string.
    Expected: Just Create New is shown.
    Actual (prior to the fix): some other scratch entries are shown. Hence, Create New is all the way at the bottom (can be even outside the view).

Related description of issue: [[Fix Scratch Creation|scratch.2021.08.31.091837.improve-lookup.fix-scratch-creation]]

Prior to this change:

pre-scratch-fix

After the change

after-scratch-fix

Pull Request Checklist

You can go to dendron pull requests to see full details for items in this checklist.

General

Quality Assurance

  • [d] add a test for the new feature, now new feature fixing a bug.

Tried to add a test for this use case but for some reason selection does not seem to be picked up as expected in a test
tried to add

  VSCodeUtils.getActiveTextEditor()!.selection =
    new vscode.Selection(7, 0,
      7, 8);

In a test after the note is opened and the selection is not picked up by the look up in a test.

https://github.com/dendronhq/dendron/blob/master/packages/plugin-core/src/test/suite-integ/NoteLookupCommand.test.ts#L721

  • make sure all the existing tests pass
  • do a spot check by running your feature with our test workspace
  • after you submit your pull request, check the output of our integration test and make sure all tests pass
    • NOTE: if you running mac/linux, check the windows output and vice versa if you are developing on windows

Docs

  • Make sure that the PR title follows our commit style
  • Please summarize the feature or impact in 1-2 lines in the PR description
  • [x ] If your change reflects documentation changes, also submit a PR to dendron-site and mention the doc PR link in your current PR

Example PR Description

# feat: capitalize all foos

This changes capitalizes all occurences of `foo` to `Foo` 

Docs PR: <URL_TO_DOCS_PR>

Copy link
Member

@kevinslin kevinslin left a comment

Choose a reason for hiding this comment

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

the way you can test for this:

  1. create a scratch note (save name to variable)
  2. create another scratch note (save name to variable)
  3. make sure second scratch note doesn't equal the first one

await wait1Second();
const scratch2Name = await createScratch();

expect(scratch1Name).toNotEqual(scratch2Name);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the requested test.

@kevinslin kevinslin changed the base branch from master to release/0.58 September 7, 2021 16:33
@kevinslin kevinslin merged commit a4f134d into release/0.58 Sep 7, 2021
@kevinslin kevinslin deleted the nk-fix-scratch-selected-text-creation branch September 7, 2021 16:33
kevinslin pushed a commit that referenced this pull request Sep 7, 2021
…match scratches just due to prefix (#1292)

* fix: creating scratch when text is selected within a note SHOULD not match scratches just due to prefix

* Add requested test
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