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

delete.term doesn't always work for dependents results #5083

Open
ceedubs opened this issue Jun 12, 2024 · 1 comment
Open

delete.term doesn't always work for dependents results #5083

ceedubs opened this issue Jun 12, 2024 · 1 comment
Assignees
Labels

Comments

@ceedubs
Copy link
Contributor

ceedubs commented Jun 12, 2024

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):

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 ceedubs added the bug label Jun 12, 2024
@sellout sellout self-assigned this Jun 12, 2024
@sellout
Copy link
Contributor

sellout commented Jun 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants