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

Path clarifications #47

Merged
merged 8 commits into from
Feb 27, 2020
Merged

Path clarifications #47

merged 8 commits into from
Feb 27, 2020

Commits on Feb 23, 2020

  1. Several significant clarifications to Path docs.

    Primarily, being commital and direct about how special values, erm...
    *aren't* -- namely, "/" is a valid segment, and so is empty string --
    and confessing how much tricky work that makes.
    
    Several methods are now more upfront about how much they *do not do
    good things* on such values.
    
    Added some new constructors for Path which *do* work for all possible
    values.  (You could've handled such tricky values with a series of
    Join calls, previously, but that would be ergonomically grueling.)
    
    These facts are all defacto 'true' to the best of my knowledge now.
    However, the IPLD specs repo is a bit on the quiet side about them
    at present (precisely because it's such an irritating little nest of
    edge cases and fun stuff)... and so the comments are also littered
    with "this may change" warnings.  Still, it's better to be accurate
    about what the code does and does not do in its current state.
    
    I'd *like* to formally specify an escaping system and canonical
    string encoding for paths.  That should start in the IPLD Specs
    repo, though, and involve fixtures, so I won't start it here now.
    
    Thanks to @ribasushi for the kick in the shins that these docs needed
    work and clarifications.
    warpfork committed Feb 23, 2020
    Configuration menu
    Copy the full SHA
    2e1845b View commit details
    Browse the repository at this point in the history
  2. Further clarification about string encoding.

    It's not very often we care about these things in Go, and when we do,
    this stance of "string is really just bytes; guard it yourself" is
    the community norm... so, these docs should *almost* go without saying.
    However, since IPLD is all about cross-language consistency, and
    we may have people from other programming language ecosystems reading
    our docs, it seems good to be as clear and explicit as possible.
    warpfork committed Feb 23, 2020
    Configuration menu
    Copy the full SHA
    f163fee View commit details
    Browse the repository at this point in the history
  3. Spelling fix.

    warpfork committed Feb 23, 2020
    Configuration menu
    Copy the full SHA
    0f7de4a View commit details
    Browse the repository at this point in the history
  4. Document an already true fact about PathSegment's zero value.

    I... honestly hadn't even realized this before; I just noticed it
    when making a refactor that would produce this end state, and thought
    "oh, that's silly".  Evidently... it's fine, since this has already
    been the case, and not been problematic!  Alright then.
    Apparently it's not too silly after all.
    warpfork committed Feb 23, 2020
    Configuration menu
    Copy the full SHA
    2d590ab View commit details
    Browse the repository at this point in the history

Commits on Feb 24, 2020

  1. Fix for empty string PathSegment.

    Yep, it makes a lot of creation by struct uglier, as you can see in the
    diff to the test file.  Fortunately?  Nobody outside the package is
    allowed to do that anyway, so, we can pretty much ignore the ergonomic.
    (It'll still look uglier if someone's looking at stuff with spew or
    go-cmp or some other library like that which peeks naughtily into
    unexported fields, but that's hard to do much about.)
    
    We could also address this by adding a discriminant field to the
    PathSegment struct.  (If we wanted to use uint internally, we'd
    probably have to do that, in fact.  Either that or start using
    some other alarmingly magical number like UINT_MAX.  Ygh.)  I didn't.
    I don't think we handle lists larger than 2 billion elements
    particularly well anyway... so, adding more words of memory to
    PathSegment to support that case seems like an entirely losing trade.
    warpfork committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    98fdaf1 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    a48c9db View commit details
    Browse the repository at this point in the history

Commits on Feb 27, 2020

  1. Add explicit documentation re null bytes to Path.

    There's already explicit sections for empty string and slash cases,
    on the basis of how likely those are to provoke questions
    (even though the content is essentially to highlight how *not*
    exceptional they are); it makes just as much sense to do the same
    call out for null bytes.
    
    Original text started in
    https://github.com/ipld/go-ipld-prime/pull/47/files#r384127483 .
    
    Co-Authored-By: Peter Rabbitson <ribasushi@leporine.io>
    warpfork and ribasushi committed Feb 27, 2020
    Configuration menu
    Copy the full SHA
    1e3e2ff View commit details
    Browse the repository at this point in the history
  2. Expand the remark on slashes in paths.

    Previous commit with suggestions from reviewer made me look at this
    line again and realize it was *too* specific.
    warpfork committed Feb 27, 2020
    Configuration menu
    Copy the full SHA
    8093be7 View commit details
    Browse the repository at this point in the history