feat(add): add support for naming pinned CIDs #10877
Conversation
There was a problem hiding this comment.
Thank you for submitting this, looks like an useful feature.
Before merging, we need:
- add short description of the new feature -- ok to reuse example from this PR -- to
docs/changelogs/v0.37.md - add tests to
test/clithat testsipfs addwith--pin-nameworks as expected- confirms
--pin-name=foocreated pin namedfoo - confirms
--pin=false --pin-name=fooproduces an error - confirms what happens if no value is passed to
--pin-name- easiest way is to always require name, and don't do any magic
- if we want to do magic, then it needs to be robus:
- if in CLI, and not piped, the source filename is used
- what happens if added file was added from stdin (
echo hi | ipfs add --pin-name)? ignore empty name?
- what happens if added file was added from stdin (
- if in HTTP RPC, we ignore empty name?
- if in CLI, and not piped, the source filename is used
- confirms
@KapilSareen mind adding tests and addressing the behaviors above and making sure the CI is green?
Signed-off-by: kapil <kapilsareen584@gmail.com>
d3d933a to
1d27b9d
Compare
|
Done @lidel! I’ve added tests in
Currently, if the input is from Also added a changelog entry under Let me know if you'd like the fallback behavior tweaked further or if there's anything else missing! |
- modify PinRoot to accept name parameter, eliminating double pinning - remove automatic filename fallback logic for cleaner behavior - only create named pins when explicitly requested via --pin-name=value - replace NoPinName constant with idiomatic empty string literals - Update help text and tests to reflect explicit-only behavior
core/commands/add.go
Outdated
| if pinName == "" { | ||
| it := req.Files.Entries() | ||
| if it.Next() { | ||
| pinName = it.Name() |
There was a problem hiding this comment.
The failing tests reveal current implementation has several problematic behaviors:
- Automatic filename as pin name: When no
--pin-nameis provided, it automatically uses filename, but filename is known only in CLI only when not piped (if piped or over HTTP, we still have no name) - Impossible
--pin-namewithout value: We cannot make--pin-namework without consuming next argument due togo-ipfs-cmdslimitations. This is potentially bad because--pin-name foowill consume "foo" but--pin-name -Q foowill consume-Q(?). In general, gets hairy. - Error-prone magic behavior: (imo) users familiar with unix tools don't expect automatic pin naming when they don't specify the flag.
I still think --pin-name=value is useful, so let's reduce scope of this PR to that, and remove magic that automatically "guesses" the value. If --pin-name is passed, user needs to provide value, or it errors.
Since I've already reviewed things, its easier if I do it – I'll apply changes shortly.
There was a problem hiding this comment.
Thank you @KapilSareen, I've simplified to require explicit ---pin-name to unblock merging this.
Lgtm, we will include it in v0.37
This PR adds support for assigning a human-readable name to a pinned CID. Named pins enhance usability by allowing users to refer to content symbolically instead of using raw multihashes.
Changes
--pin-nameflag toipfs addExample Usage
Fixes #10341