Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Allow CID functions to work with non-default multibase. #9

Closed
wants to merge 1 commit into from

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Sep 28, 2018

This:

  1. Chaned the FromCid function to take a function which translates a Cid to s string.
  2. Fixes ParseCidToPath to preserve the original CID string after verifying it is a correct CID.

Yes, (1) is an API change but I greatly prefer it to creating another another tediously named function. This function is not used in many places within go-ipfs. In almost all cases we should be passing in a transformer function anyway, and when we don't have one nil can be used instead. The general idea is we should eventually get the transformer function from the current context which is nearly always available when FromCid is used. A function is used instead of just a multibase encoding for added flexibility, for example we may want to upgrade CidV0 to CidV1.

CC @Stebalien

@Stebalien
Copy link
Member

What about passing in formatting options. That is, turn this into:

func FromCid(c cid.Cid, options ...CidFormatOption)

(or something like that)

Yes, it'll probably be slightly more complicated but its extensible and nicer for the user. I can then, e.g., write FromCid(c, CidBase(someMultibase), ...). Thoughts?

@Stebalien
Copy link
Member

Basically, forcing users to pass a CID "transformer" in on every call to FromCid is just kind of icky.

@kevina
Copy link
Contributor Author

kevina commented Sep 28, 2018

In my view this is not the place for complicated option parsing as that should be done higher up. The idea is we set a single value in the context (using context.WithValue) higher up and then get the value from the context using cidenc.Get(ctx) (see ipfs/go-cidutil#8).

Part of the reason I took in a simple function (rather than say an interface) is to avoid a circular decency with go-cidutil that will be introduced once ipfs/go-cidutil#8 is merged.

@kevina
Copy link
Contributor Author

kevina commented Sep 28, 2018

Or to put it another way the idea is to use the cidenc package (from ipfs/go-cidutil#8) to construct an encoder and then pass that to FromCid. This encoder will often be constructed higher up and stashed into the context. I would take an interface but depending on cidenc will create a circular decency that needs to be resolved somehow. (The cidenc package has a utility function that parses a path for a Cid in order to set default paramaters based on an existing Cid or path, which is used by ipfs resolve to fix ipfs/kubo#5234).

@Stebalien Stebalien closed this Jan 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants