fix(settings): name the context the wipe screen tells you to delete - #284
Merged
Conversation
The wipe-profile screen ends by saying the VTA-side context, persona DID and keys survive, and to run `pnm contexts delete` if you want those gone too. It named no context — on the one screen that is about to remove the config file and the keyring entry that hold the id, which is why the advice says "first". Sending the operator away to look it up was sending them somewhere that stops existing a keypress later. The id is now carried on `SettingsState` from `account.top_context_id`, set in the same `sync_from_config` pass as the rest of the pane, and the command is printed complete on its own row — the treatment every other command this TUI hands over already gets, because it is meant to be retyped in another terminal. Positional, as `pnm-cli`'s `ContextCommands` defines it; the `--did` flag that had to come off the identity pane's `pnm acl update` hint in #283 is the same mistake one repo boundary away, so the shape is asserted rather than assumed. A nested context keeps its whole `<parent>/<id>` path, since `pnm` addresses a sub-context that way and the leaf alone names a different context or none. An unloaded account keeps a placeholder: no id is better than a wrong one in a command that deletes a context and everything under it. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #283, same class of defect one screen over.
The wipe-profile screen ends by saying what survives, and what to run if you
want that gone too:
It named no context — on the one screen that is about to remove the config file
and the keyring entry that hold the id. That is exactly why the advice says
first: sending the operator away to look the id up was sending them somewhere
that stops existing a keypress later.
After
The id rides on
SettingsStatefromaccount.top_context_id, set in the samesync_from_configpass as the rest of the pane. The command gets its own row —the treatment every other command this TUI hands over already gets, because it
is meant to be retyped in another terminal.
Three details worth the review
pnm-cli'sContextCommands::Deletedefines it. The--didflag that had to come off the identity pane'spnm acl updatehint infeat(health): print the DID this install authenticates to the VTA as #283 is the same mistake one repo boundary away, so the shape is asserted in a
test rather than assumed (R3.6).
<parent>/<id>path —pnmaddresses asub-context that way, and the leaf alone names a different context or none.
one in a command that deletes a context and everything under it.
Testing
Five unit tests over the rendered screen and the command builder: the id is
named, the placeholder is gone, the flag shape holds, a nested path survives
whole, an empty id falls back, and the screen still says what the wipe does
not touch — the sentence that makes the command needed in the first place.
cargo fmt --all --check,cargo clippy --all-targets --all-features -D warningsRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featurescargo test --all-features --workspace— 446 passed inopenvtc, workspace greenPre-merge checklist