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

Add support for compressing arbitrary URLs. #75

Closed
wants to merge 1 commit into from
Closed

Conversation

davidlehn
Copy link
Member

Add support for compressing arbitrary URLs. Any id which is in the appContextMap will be compressed as a single number before using other URL codecs.

  • Need feedback if this is a good feature and approach.
  • The appContextMap naming is being overridden here and elsewhere. This feature could change and use another map.
  • The sub-codec is value is important. 1026 is encoded as 3 bytes. A low value would be 1 or 2 bytes. If lots of URLs get encoded this would add up.
  • This was modified from other codecs, it may be able to be cleaned up if not handling fallback cases.
  • Needs tests.

@davidlehn davidlehn force-pushed the app-url-codec branch 2 times, most recently from 29bf4e1 to ce06067 Compare December 21, 2023 04:02
[1025, Base58DidUrlDecoder]
[1025, Base58DidUrlDecoder],
// FIXME: encdeds as 3 bytes, may want to use a lower id
[1026, AppUrlDecoder]
Copy link
Member

Choose a reason for hiding this comment

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

Seems like we could use 4 here without issue, right?

Copy link
Member Author

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 the policy is here. This is another registry of ids. This app url codec seems general enough to use a limited low id. Perhaps more specialized codecs don't? I'm guessing that's why the base58did ones are higher?


### Added
- Add support for compressing arbitrary URLs. Any id which is in the
`appContextMap` will be compressed as a single number before using other URL
Copy link
Member

Choose a reason for hiding this comment

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

We should add an alias for appContextMap and start using it -- and deprecate the name appContextMap and remove it in the next version.

Maybe appTermMap, appTermIdMap, or appStringMap ... open to alternatives.

Add support for compressing arbitrary URLs. Any id which is in the
`appContextMap` will be compressed as a single number before using other
URL codecs.
@dlongley
Copy link
Member

I'm closing this because PR #87 supersedes it.

@dlongley dlongley closed this Jul 11, 2024
@dlongley dlongley deleted the app-url-codec branch July 11, 2024 21:54
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