edit should? comment out generated record functions

[ceedubs]

Problem/context

When you have both a record type and its associated generated functions in your scratch file, you get compile errors are multiple bindings.

One reason that this is annoying is that unless you've made changes, the functions in the scratch file should match those that would be generated for the record type.

Generally you don't want to edit the generated functions directly; you want to change the record type and have the new versions automatically generated.

This comes up a lot because currently a common way to work on a project is to call find and then edit 1-192 to edit the entire project in a scratch file.

Related: #4004

Example transcript

.> builtins.merge

  Done.

structural type Foo = {
  bar : Nat,
  baz : Text
}

  I found and typechecked these definitions in scratch.u. If you
  do an `add` or `update`, here's how your codebase would
  change:
  
    ⍟ These new definitions are ok to `add`:
    
      structural type Foo
      Foo.bar        : Foo -> Nat
      Foo.bar.modify : (Nat ->{g} Nat) -> Foo ->{g} Foo
      Foo.bar.set    : Nat -> Foo -> Foo
      Foo.baz        : Foo -> Text
      Foo.baz.modify : (Text ->{g} Text) -> Foo ->{g} Foo
      Foo.baz.set    : Text -> Foo -> Foo

.> add

  ⍟ I've added these definitions:
  
    structural type Foo
    Foo.bar        : Foo -> Nat
    Foo.bar.modify : (Nat ->{g} Nat) -> Foo ->{g} Foo
    Foo.bar.set    : Nat -> Foo -> Foo
    Foo.baz        : Foo -> Text
    Foo.baz.modify : (Text ->{g} Text) -> Foo ->{g} Foo
    Foo.baz.set    : Text -> Foo -> Foo

.> find Foo

  1. structural type Foo
  2. Foo.bar : Foo -> Nat
  3. Foo.bar.modify : (Nat ->{g} Nat) -> Foo ->{g} Foo
  4. Foo.bar.set : Nat -> Foo -> Foo
  5. Foo.baz : Foo -> Text
  6. Foo.baz.modify : (Text ->{g} Text) -> Foo ->{g} Foo
  7. Foo.baz.set : Text -> Foo -> Foo
  8. Foo.Foo : Nat -> Text -> Foo
  

.> edit 1-8

  ☝️
  
  I added these definitions to the top of
  /home/cody/code/unison/scratch.u
  
    structural type Foo
      = { bar : Nat, baz : Text }
    
    Foo.bar : Foo -> Nat
    Foo.bar = cases Foo bar _ -> bar
    
    Foo.bar.modify : (Nat ->{g} Nat) -> Foo ->{g} Foo
    Foo.bar.modify f = cases Foo bar baz -> Foo (f bar) baz
    
    Foo.bar.set : Nat -> Foo -> Foo
    Foo.bar.set bar1 = cases Foo _ baz -> Foo bar1 baz
    
    Foo.baz : Foo -> Text
    Foo.baz = cases Foo _ baz -> baz
    
    Foo.baz.modify : (Text ->{g} Text) -> Foo ->{g} Foo
    Foo.baz.modify f = cases Foo bar baz -> Foo bar (f baz)
    
    Foo.baz.set : Text -> Foo -> Foo
    Foo.baz.set baz1 = cases Foo bar _ -> Foo bar baz1
  
  You can edit them there, then do `update` to replace the
  definitions currently in this namespace.

.> load scratch.u

  ❗️
  
  I found multiple bindings with the name Foo.bar:
      2 |   = { bar : Nat, baz : Text }
      3 | 
      4 | Foo.bar : Foo -> Nat
      5 | Foo.bar = cases Foo bar _ -> bar
  
  
  I found multiple bindings with the name Foo.bar.modify:
      2 |   = { bar : Nat, baz : Text }
      3 | 
      4 | Foo.bar : Foo -> Nat
      5 | Foo.bar = cases Foo bar _ -> bar
      6 | 
      7 | Foo.bar.modify : (Nat ->{g} Nat) -> Foo ->{g} Foo
      8 | Foo.bar.modify f = cases Foo bar baz -> Foo (f bar) baz
  
  
  I found multiple bindings with the name Foo.bar.set:
      2 |   = { bar : Nat, baz : Text }
      .
     11 | Foo.bar.set bar1 = cases Foo _ baz -> Foo bar1 baz
  
  
  I found multiple bindings with the name Foo.baz:
      2 |   = { bar : Nat, baz : Text }
      .
     14 | Foo.baz = cases Foo _ baz -> baz
  
  
  I found multiple bindings with the name Foo.baz.modify:
      2 |   = { bar : Nat, baz : Text }
      .
     17 | Foo.baz.modify f = cases Foo bar baz -> Foo bar (f baz)
  
  
  I found multiple bindings with the name Foo.baz.set:
      2 |   = { bar : Nat, baz : Text }
      .
     20 | Foo.baz.set baz1 = cases Foo bar _ -> Foo bar baz1
  

Proposed solution

I think that a simple solution is to make edit comment out generated record functions. Something like this:

-- This is a generated function for a record type. Did you mean to edit the record type directly?
-- Foo.bar.modify : (Nat ->{g} Nat) -> Foo ->{g} Foo
-- Foo.bar.modify f = cases Foo bar baz -> Foo (f bar) baz

[aryairani]

Erp, thanks for the report

[aryairani]

Or I wonder how hard it would be to — if two definitions are the same in the scratch file, ignore one

[ceedubs]

@aryairani I'd still prefer commenting them out because if you change the record type you don't want to change (or delete) 3 auto generated functions that are in your scratch file.

[aryairani]

@ceedubs If we had an edit.namespace that didn't put accessors into the file explicitly, would that resolve this issue?

[aryairani]

Alternatively, if you are editing a type and have an accessor for it in the scratch file, I think your custom accessor should take priority over an auto-generated one, which I don't think is what you want in your example where you're maybe editing the types and only edited the accessor by accident, right?

[ceedubs]

@aryairani yes if there were an edit.namespace that didn't put them into the file that would work. I just got the impression that people didn't really like my #4004 proposal and that it might never happen.

[aryairani]

from https://github.com/unisonweb/unison/pull/4796#issuecomment-2002625062

Except unrelated, edit.namespace dumps all the record accessor functions to the file and then UCM complains about duplicates - do we have a bug tracking this already? But once I removed all the duplicates everything round tripped with the same hash.

We're fixing this issue where it comes up for merge, and then it'll hopefully be straightforward to apply the same logic to edit and edit.namespace.

[aryairani]

Calling this closed with #5034 but reopen or create a new issue if something's missing