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 orphan, placeholder, tag-explorer counts #1131

Merged

Conversation

badsketch
Copy link
Contributor

I think there's a slight bug with change 7a2756e, where the counts are out of sync with the actual number of entries

To reproduce in new project:

  1. create note1
  2. create note2 and add a placeholder [[note3]]
  3. note that orphans panel says Orphans (2) even though it only has note1 and placeholders panel says Placeholders (0) with the note3 entry
    Screenshot 2023-01-03 102024

I believe we just need to tweak the subscription logic so the panel title is set after we do the calculations for each panel.

treeView.title = baseTitle + ` (${foam.tags.tags.size})`;
})
);
foam.tags.onDidUpdate(() => provider.refresh());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually didn't run into issues with tag count becoming out of sync during my testing, but I still made this change as I believe there could be a race condition. The title could be wrong if provider.refresh() doesn't complete before title is generated. Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no race condition here because we don't use the provider to compute the number, but the foam.tags object directly (also this is the reason why you didn't run into issues in the first place)

Copy link
Collaborator

@riccardoferretti riccardoferretti left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this, your change makes sense, it's good to go

treeView.title = baseTitle + ` (${foam.tags.tags.size})`;
})
);
foam.tags.onDidUpdate(() => provider.refresh());
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no race condition here because we don't use the provider to compute the number, but the foam.tags object directly (also this is the reason why you didn't run into issues in the first place)

@riccardoferretti riccardoferretti merged commit 0b879c0 into foambubble:master Jan 4, 2023
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