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

implement indexing + querying over canvas files #1803

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

GamerGirlandCo
Copy link
Contributor

fixes #1739
by the way, if anyone has any suggestions for improvements, please let me know!

mindex: CanvasMetadataIndex
) {
// @ts-expect-error SHUT UP MEG
const metadata = mindex[path]?.caches[id];
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure what error this is, but if it's a null check you can use ! to assert that the result exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the error is:

Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'CachedMetadata'.
No index signature with a parameter of type 'string' was found on type 'CachedMetadata'.ts(7053)

src/data-import/web-worker/import-manager.ts Outdated Show resolved Hide resolved
src/data-index/index.ts Outdated Show resolved Hide resolved
@blacksmithgu
Copy link
Owner

Looks great! I mostly just have a few tslint nits and then one remark about the code in FileImporter which may need to be slightly adjusted.

add calllbacks to `metadataCache.changed` and `metadataCache.resolved`
also move the registerevent callback argument into a separate function to avoid repetition
@daamiian
Copy link

When might this functionality be rolled out?

@GamerGirlandCo
Copy link
Contributor Author

GamerGirlandCo commented May 28, 2023

When might this functionality be rolled out?

¯\_(ツ)_/¯

@blacksmithgu
Copy link
Owner

Howdy @GamerGirlandCo, I'd actually like to add this functionality to Datacore where I think the data model is a more natural fit for supporting canvas files + canvas pages. Happy to discuss how it should look like.

@HananoshikaYomaru
Copy link

@blacksmithgu so sound like this PR is not going to merge in dataview. We need to wait for datacore?

@GottZ
Copy link
Contributor

GottZ commented Mar 25, 2024

hm. in regards to b201bd1 I'd actually prefer not to see politics in commit messages.
especially, since I don't want to see this repo getting DMCA'd for something like that.
(talking out of experience.. I was hosting git repos for a couple communities before and once, a revoked commit got a repo a dmca. I had to re-write history and force-push master to change it)

@blacksmithgu
Copy link
Owner

I squash merge and usually erase any interim commit history for PRs.

@ctwhome
Copy link

ctwhome commented Mar 29, 2024

Hi @blacksmithgu you referred above that this feature would be likely a better suit for Datacore, but in the meantime, it could be really useful if it gets integrated into Dataview, especially now there is an almost done PR for it. Would that be still the plan?

@GamerGirlandCo
Copy link
Contributor Author

hm. in regards to b201bd1 I'd actually prefer not to see politics in commit messages.
especially, since I don't want to see this repo getting DMCA'd for something like that.
(talking out of experience.. I was hosting git repos for a couple communities before and once, a revoked commit got a repo a dmca. I had to re-write history and force-push master to change it)

it's not politics it's history :)

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.

Index .canvas files
6 participants