unison icon indicating copy to clipboard operation
unison copied to clipboard

Make `delete` understand suffixified names

Open mitchellwrosen opened this issue 1 year ago • 2 comments

delete doesn't seem to accept suffixes.

I had a type I knew I wanted to delete, which still had a bunch of dependents:

mbta/main> dependents predictions.Request

  Dependents of: type predictions.Request

    Terms:

    1.  Request.filterLatitude
    2.  Request.filterLongitude
    3.  Request.filterRadius
    4.  Request.filterRevenue
    5.  Request.filterRoutePattern
    6.  Request.filterRouteType
    7.  Request.filterStop
    8.  Request.filterTrip
    9.  Request.include
    10. Request.pageLimit
    11. Request.pageOffset
    12. Request.sort
 
  Tip: Try `view 12` to see the source of any numbered item in the above list.

Yet a delete 1-12 didn't work:

mbta/main> delete 1-12
mbta/main> delete Request.filterLatitude Request.filterLongitude Request.filterRadius Request.filterRevenue Request.filterRoutePattern Request.filterRouteType Request.filterStop Request.filterTrip Request.include Request.pageLimit Request.pageOffset Request.sort

  ⚠️

  The following names were not found in the codebase. Check your spelling.
    Request.filterLatitude
    Request.filterLongitude
    Request.filterRadius
    Request.filterRevenue
    Request.filterRoutePattern
    Request.filterRouteType
    Request.filterStop
    Request.filterTrip
    Request.include
    Request.pageLimit
    Request.pageOffset
    Request.sort

mitchellwrosen avatar Jul 12 '24 21:07 mitchellwrosen

Hm. Yeah it would be okay to have delete accept suffixified names, provided that it prints the full names in the output, and that undo is quick and reliable (it's at least reliable now afaik, and possible also quick?).

It strikes me as separately wrong though, that the numbered arg is a suffixified name instead of a fully qualified one. Without looking at the code, I'm not sure whether this is an issue on the dependents end or the delete end or some other layer in between, but I feel like the use case demonstrated in this ticket should have succeeded regardless of whether delete accepts suffixified names or not.

aryairani avatar Jul 13 '24 03:07 aryairani

It strikes me as separately wrong though, that the numbered arg is a suffixified name instead of a fully qualified one.

I thought this was covered in another ticket somewhere, but if so, I can’t find it.

The gist being that we should have absolute (or project-rooted) names basically everywhere, and then the pretty-printed numbered lists are suffixed “on demand”. E.g., find.global might suffix names relative to the current project, while ls would suffix relative to the provided namespace. But numberedArgs would hold “unique” (absolute or hash-qualified[^1]) names in all cases.

[^1]: here I mean “definitely includes a hash”, not HashQualified.

Without looking at the code, I'm not sure whether this is an issue on the dependents end or the delete end or some other layer in between, but I feel like the use case demonstrated in this ticket should have succeeded regardless of whether delete accepts suffixified names or not.

This looks like an issue in handleDependents – it seems like it has full names, but sets numberedArgs using “short names”. This could be a quick fix (since the pretty-printed list can still be given the short names).

However, there are also operations on the other end (e.g., alias.many) that only accept relative names, and so if we start always returning absolute paths, but displaying suffixed names, those commands may not work as expected (e.g., making the aliased name in the wrong namespace) or at all.

So, I think the right way to go about this is to first change the operations that require relative names to not, and make sure the semantics are consistent regardless of how prefixed the names are. Then we can make sure that numberedArgs entries are always absolute (or contain a hash).

sellout avatar Jul 18 '24 02:07 sellout