unison icon indicating copy to clipboard operation
unison copied to clipboard

delete.term doesn't always work for dependents results

Open ceedubs opened this issue 1 year ago • 1 comments

Describe and demonstrate the bug

If you run dependents and then try to delete.term with a numbered result it doesn't work (or at least not always).

Below you can see that it works if I manually select the fully-qualified name.

Screenshots

@cloud/nimbus/cleanup-2> dependents up.codec.Decode.variableSizeBytes
 
  Dependents of: codec.Decode.variableSizeBytes
  
    Terms:
  
    1. variableSizeBytes.tests.success
  
  Tip: Try `view 1` to see the source of any numbered item in the above list.

InputPattern: 937 ms (cpu), 937 ms (system)
@cloud/nimbus/cleanup-2> delete.term 1
@cloud/nimbus/cleanup-2> delete.term variableSizeBytes.tests.success
 
  ⚠️
  
  The following names were not found in the codebase. Check your spelling.
    variableSizeBytes.tests.success

InputPattern: 572 µs (cpu), 2.07 ms (system)
@cloud/nimbus/cleanup-2> delete.term    
Select a definition to delete:
@cloud/nimbus/cleanup-2> delete.term up.codec.encode.variableSizeBytes.tests.success

updateRoot: 48.9 ms (cpu), 37.7 ms (system)
 
  Done.

Environment (please complete the following information):

  • ucm --version 335512e331339cd496db97b6d3bead1efc090f42

Additional context

It's possible that this is a regression that was introduced at the same time that #5055 was, but I haven't verified that. cc @sellout

ceedubs avatar Jun 12 '24 03:06 ceedubs

dependents returns numbered arguments as Name, and delete.term (and all the delete.* commands) expects HQSplit'. The conversion should be (and appears to be) lossless.

Unfortunately, dependents foo; delete 1 always fails prior to the structured arguments work because of #4898, so it’s hard to tell if this particular problem existed before.

transcript
.> project.create test-5083
test-5083/main> builtins.merge
foo = 1 + 1
meh.bar = foo + foo
baz = foo + bar
test-5083/main> add
test-5083/main> find bar
test-5083/main> dependents foo
test-5083/main> debug.numberedArgs

The results of dependents are Names.

We can easily delete baz based on the dependents results.

test-5083/main> delete.term 2

We can view bar from the dependents results (which expects a HashQualified Name).

test-5083/main> view 1

but deleting (which expects a HQSplit') bar (which is in a deeper namespace) fails.

test-5083/main> delete.term 1

rename.term, which also expects a HQSplit', also fails.

test-5083/main> rename.term 1 quux

Using the expanded term also fails.

test-5083/main> delete.term bar

But adding the missing namespace works.

test-5083/main> delete.term meh.bar

The issue seems to be that the Names returned by dependents aren’t qualified enough for HQSplit'-consuming commands to find them (understandable that we don‘t want delete to delete some random term with the same name that happens to be found).

I don’t think this is related to the numbered args changes, because I don’t see anything relevant around where dependentsNames are produced.

I think this is another face of #1298 – if we had absolute (or hash-qualified) names produced, there would be no confusion/ambiguity when they’re presented to delete.term.

sellout avatar Jun 12 '24 21:06 sellout