Add completion spec: ip link - #319
Conversation
Fill in the `ip link` signature in json/ip.json, which previously carried only a name and a description. Covers the actions `add`, `delete`, `set`/ `change`, `show`/`list`/`lst`, `xstats`, `afstats`, `property` and `help`, along with their options and arguments: device selection, up/down, mtu, rename, address, txqueuelen, master/nomaster, netns, link types for add, the SR-IOV `vf` parameters, the XDP attach options and the `inet` devconf parameters. Device and namespace arguments reuse the existing `network_interfaces` and `netns` generators. Grounded in ip-link(8) (iproute2 man/man8/ip-link.8.in) and iproute2's own `ip link help` output (ip/iplink.c). Co-Authored-By: Warp Agent <agent@warp.dev>
|
This PR was generated with Warp. |
Six fixes, each verified against iproute2 main: - `ip link property delete` is not a keyword ip accepts. iplink_prop() dispatches on matches(*argv, "del"), and matches() only succeeds when the token is a prefix of the pattern, so "delete" exits with "Operator required". The top-level `ip link delete` uses matches(*argv, "delete") and is unaffected. - `state` on set completes at the device position, where iplink_parse() routes it to get_operstate(); auto/enable/disable are only valid after `vf NUM`. Retarget it to the oper_states[] value list and note the virtual-function meaning in its description. - `xstats type` only works for the four link helpers that define .print_ifla_xstats: bond, bond_slave, bridge, bridge_slave. Every other type exits with "link type X doesn't support xstats". - Add `batadv` to the link type lists. It is registered by ip/iplink_batadv.c just like can and lowpan, which the man page lists. - Add the `qlen` alias, accepted by ip/iplink.c alongside txqueuelen and txqlen. - `property add|del` take a bare positional DEVICE, since iplink_prop_mod() treats any non-altname token as the device; mark altname isRepeatable. Co-Authored-By: Warp Agent <agent@warp.dev>
ip/link_vti6.c registers a link_util with a .parse_opt, so vti6 is creatable in the same way as batadv, can and lowpan, none of which appear in a documented TYPE list either. tun and vrf_slave also have helpers but are left out: theirs define only .print_opt, so they exist to render a kind in `ip link show` and cannot be created with `ip link add type ...`. Suggesting them would offer something ip rejects. The xstats list is unaffected; it stays restricted to the four types whose link_util defines .print_ifla_xstats. Co-Authored-By: Warp Agent <agent@warp.dev>
There was a problem hiding this comment.
Overview
Fills in the previously empty ip link entry in command-signatures/json/ip.json with the link actions, their options and value lists — one file, no Rust. The data was checked keyword-by-keyword against iproute2 main and three position-dependent errors were corrected before this landed in front of you; what remains below are two calls that need a human, not corrections.
Concerns
- 17 keywords are offered on
ip link setbut only parse in a nested position (inline comments below). The vf group is only read insideiplink_parse_vf()aftervf NUM,qos/protoonly aftervf NUM vlan VLANID, and the five XDP sub-keywords only afterxdp*; standalone, each one hits the parser fallthrough and errors. The Fig schema can't scope a keyword to a preceding option, so the choice is between a discoverable-but-noisy list and only suggesting what parses — the descriptions do name the requiredvfcontext, which softens the vf group, but the five XDP keywords have no standalone reading at all andobject/pinnedare already reachable through thexdpoption's own suggestions. - The repo's mandatory-screenshot rule may or may not apply here.
.agents/skills/review-pr-local/SKILL.md:20-31namesgeneratorNamereferences in changedjson/<command>.jsonfiles as a trigger, and this PR adds many; under a strict reading that forcesRequest changes. The rule's intent — prove new generator code produces completions — isn't engaged, sincenetwork_interfacesandnetnsare unchanged and already exercised byip ruleandip monitor, so this should be decided rather than left to grep.
Verdict
Checks: build pass, tests pass (133 + 10), CI pending on this head and green on the previous one, visual proof missing (see Concerns)
Found: 0 critical, 1 important, 0 suggestions, 0 nits, 1 question
Responding as wilson: Open session · View factory task
| } | ||
| }, | ||
| { | ||
| "name": "mac", |
There was a problem hiding this comment.
mac, vlan, qos, proto, rate, max_tx_rate, min_tx_rate, spoofchk, query_rss, trust, node_guid and port_guid are parsed only inside iplink_parse_vf(), i.e. after vf NUM — and qos/proto only after vf NUM vlan VLANID, via the PREV_ARG() rewind at ip/iplink.c:307,318. Offered at ip link set dev eth0 <TAB> they fall through to either "dev" is duplicate, or "mac" is a garbage. Their descriptions do name the vf requirement, so this is the milder half of the tradeoff in the review body.
| } | ||
| }, | ||
| { | ||
| "name": "object", |
There was a problem hiding this comment.
object, pinned, section, program and verbose are parsed only after xdp/xdpgeneric/xdpdrv/xdpoffload (ip/iplink_xdp.c, usage at ip/iplink.c:96-102). Unlike the vf keywords these have no standalone reading to describe, and object/pinned are already reachable through the xdp option's own suggestions directly above — so dropping these five loses nothing.
|
Closing this in favour of #318, which covers the same issue and models the data correctly. This PR puts every bare That also retracts the first concern in my review above: the 17 position-dependent keywords were an artefact of the flat Two things here are worth keeping, and I've left them on #318: the verified 32-parameter Responding as wilson: Open session · View factory task |


Fills in the
ip linksignature incommand-signatures/json/ip.json. It previously carried only a name and a description, soip link <TAB>offered nothing beyond the object name itself.Addresses warpdotdev/warp#9768.
What's covered
Actions:
add,del/delete,set/change,show/list/lst,xstats,afstats,property(add/del), andhelp.Options and arguments, including device selection (
dev,link,parentdev,group),up/down,mtu,name,address,broadcast/brd,txqueuelen/txqlen/qlen,index,numtxqueues/numrxqueues, the GSO/GRO sizing knobs,netns,link-netns/link-netnsid,alias,master/nomaster/vrf,addrgenmode, the flag toggles (arp,promisc,multicast,allmulticast,trailers,dynamic,carrier,protodown,protodown_reason), the SR-IOVvfparameters, the XDP attach options, theinetdevconf parameters, and theshowfilters (up,down,master,vrf,type,nomaster,novf).Link types are suggested for
typeonadd,delete,set,showandhelp.xstats typeis restricted tobond,bond_slave,bridgeandbridge_slave, the only four link helpers that define.print_ifla_xstats— every other type exits withlink type X doesn't support xstats.Device and namespace arguments reuse the existing
network_interfacesandnetnsgenerators, so no new Rust generators were needed.Sources
Every entry is grounded in one of:
ip-link(8)— iproute2man/man8/ip-link.8.inip link helpoutput —iplink_usage()/iplink_types_usage()inip/iplink.c, plus the argument parsers inip/iplink.c,ip/iplink_xstats.c, andip/ipaddress.c(ipaddr_list_flush_or_save, which handles theip link showfilters)iproute2could not be installed in the build environment (no root), soip link helpwas read from its source rather than executed.Where the two sources disagree, the parser wins, since it decides what
ipactually accepts. Three cases came up:peer LLADDRESSis not shipped.ip-link(8)lists it as a synonym forbroadcast/brd, butiplink_parse()only accepts prefix-matchedbroadcastand the exact stringbrd.ip link property deleteis not shipped, onlydel.iplink_prop()dispatches onmatches(*argv, "del"), andmatches()only succeeds when the token is a prefix of the pattern, so the longerdeleteexits withOperator required. The top-levelip link deleteis different — it usesmatches(*argv, "delete")— so both spellings are offered there.batadvandvti6are shipped even though neither appears in a documentedTYPElist. The rule behind the type lists is: a type is suggested if it is documented iniplink_types_usage()/ip-link(8), or creatable with alink_utilhelper. That admitsbatadv(ip/iplink_batadv.c) andvti6(ip/link_vti6.c), which are registered exactly likecanandlowpan— types the man page does list. It excludestunandvrf_slave, which have helpers too, but ones that define only.print_opt: they exist soip link showcan render those kinds, and neither can be created withip link add type ....Position-dependent keywords
stateparses differently depending on where it appears. At the device positioniplink_parse()routes it toget_operstate(); only aftervf NUMdoes it takeauto/enable/disable. Since the completion fires at the device position, the entry carries the operstate value list, and its description notes the virtual-function meaning. This is the one keyword sourced from the parser rather than the docs.More generally, the
vfparameters and the XDP sub-keywords (object,pinned,section,program,verbose) only parse aftervf NUMorxdp*respectively, but the Fig schema has no way to nest options under an option, so they are flattened ontosetand their descriptions say which keyword they follow. Flagging that tradeoff explicitly for review.Deliberately left out
ARGS(type vlan id ...,type vxlan ...,type bridge ..., and the ~40 other link types). That is a much larger surface with its own man-page sections and is better handled as follow-up work;typestill suggests the type names.ip link replace, and themodekeyword onset. They exist inip/iplink.cbut appear in neitherip link helpnorip-link(8).macaddronset— it is amacvlan/macvtaptype-specific argument, so it belongs with the per-type work above.Validation
script/presubmitpasses:npm run format:check— all matched files use Prettier code stylecargo fmt -p warp-command-signatures -p warp-completion-metadata --checkcargo clippy -p warp-command-signatures -p warp-completion-metadata --all-targets --all-features -- -D warningscargo test --verbose— 133 passed inwarp-command-signatures(this includes the invariants that every spec deserializes and that every referenced generator name exists), 10 passed inwarp-completion-metadata, 0 failedFollow-up
Once this merges,
warp-command-signaturesneeds a rev bump inwarpdotdev/warp'sCargo.toml(currently pinned tofe35266) before the completions reach the app. That PR is not opened yet, since the rev does not exist until this lands.