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

feature/improved prefix handling #262

Merged
merged 3 commits into from
Apr 27, 2022

Conversation

abeforgit
Copy link
Member

@abeforgit abeforgit commented Apr 26, 2022

Result of a rabbithole that ended nowhere while debugging #261

  • Collect seen prefixes on parse
  • Allow users to specify extra prefixes ad-hoc

@abeforgit abeforgit added the enhancement New feature or request label Apr 26, 2022
@abeforgit abeforgit requested review from nvdk and lagartoverde April 26, 2022 22:03
@abeforgit abeforgit force-pushed the feature/improved-prefix-handling branch from a058ea1 to 840957e Compare April 26, 2022 22:07
@nvdk
Copy link
Member

nvdk commented Apr 27, 2022

Result of a rabbithole that ended nowhere while debugging #262

I think you might be linking to the wrong issue/PR here :). I'm not sure what this solves or offers other than a warning when overwriting prefixes with a different value (which may be valuable in itself).

@abeforgit
Copy link
Member Author

Result of a rabbithole that ended nowhere while debugging #262

I think you might be linking to the wrong issue/PR here :). I'm not sure what this solves or offers other than a warning when overwriting prefixes with a different value (which may be valuable in itself).

Ah yes I meant 261, edited.
This doesnt really solve anything, but I made some wrong assumptions yesterday that the problems we were having were related to the way prefixes are handled. This was an area with a mental "todo" already since it's been an awkward implementation from the start.

Basically the "concise term" logic allowed users of the datastore to specify terms in a natural way such as "prefix:predicate", but the prefixes it understood were not related to the ones defined in the document. This fixes that and also allows to specify extra prefixes should that ever be necessary.

So this is more of a "well im here anyway, might as well clean this up" pr

Copy link
Member

@nvdk nvdk left a comment

Choose a reason for hiding this comment

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

didn't test the code specifically but didn't encounter any issues and the code looks sound

@abeforgit abeforgit merged commit d20b8bf into development Apr 27, 2022
@abeforgit abeforgit deleted the feature/improved-prefix-handling branch April 27, 2022 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants