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

Preserve record syntax in view and edit #2260

Closed
hojberg opened this issue Jul 22, 2021 · 4 comments · Fixed by #2403
Closed

Preserve record syntax in view and edit #2260

hojberg opened this issue Jul 22, 2021 · 4 comments · Fixed by #2403
Labels
dx Developer experience improvements or workflow fixes E1 Lowest effort/time I2 Medium impact pretty-printer R3 High reach

Comments

@hojberg
Copy link
Member

hojberg commented Jul 22, 2021

Record syntax are sugar that helps create a bunch of accessor functions, but the syntax is lost when viewed or edited. This makes it very hard to:

  1. Even know this was a record in the first place, thus forgetting about the accessor functions and the records field names
  2. Editing a record—say, change a field from String to Nat—without the record syntax makes dealing with the accessor functions incredibly frustrating and as soon as the developer replicates the record syntax, this pain goes away, but again—they'll have to remember the names exactly to avoid making mistakes.

The goal of this ticket is to come up with a record solution.

@hojberg hojberg added design needed pretty-printer dx Developer experience improvements or workflow fixes R3 High reach I2 Medium impact labels Jul 22, 2021
@hojberg hojberg added this to the Beta milestone Jul 22, 2021
@pchiusano
Copy link
Member

There is actually some logic for this and I remember it working at one point. It just needs some debugging.

-- Comes up with field names for a data declaration which has the form of a
-- record, like `type Pt = { x : Int, y : Int }`. Works by generating the
-- record accessor terms for the data type, hashing these terms, and then
-- checking the `PrettyPrintEnv` for the names of those hashes. If the names for
-- these hashes are:
--
--   `Pt.x`, `Pt.x.set`, `Pt.x.modify`, `Pt.y`, `Pt.y.set`, `Pt.y.modify`
--
-- then this matches the naming convention generated by the parser, and we
-- return `x` and `y` as the field names.
--
-- This function bails with `Nothing` if the names aren't an exact match for
-- the expected record naming convention.
fieldNames
  :: forall v a . Var v
  => PrettyPrintEnv
  -> Reference
  -> HashQualified Name
  -> DataDeclaration v a
  -> Maybe [HashQualified Name]

If I had to guess, the logic is too fragile if it's given a suffixified pretty print environment.

@hojberg
Copy link
Member Author

hojberg commented Sep 10, 2021

Some more details on this. It might be that it only breaks if there are more than 2 fields?

@hojberg
Copy link
Member Author

hojberg commented Sep 14, 2021

I made a transcript and it fails for all the examples I gave it (regardless of number of fields). There is a case where records do work, but I haven't been able to consistently find it.

Here's the transcript: https://gist.github.com/hojberg/3d55566ac55cc76314dc73119bd63a07
They all result in view (and thus edit) without the curly record syntax.

So unique type MyRecord = { a : Nat, b : Text } when viewed becomes unique type MyRecord Nat Text which is not helpful.

cc @runarorama

@aryairani
Copy link
Contributor

aryairani commented Sep 14, 2021

I'm bumping into the same fieldNames function for #2373, as it does some hashing. It looks like it generates the accessor code and then hashes it, and looks for matching names in the PPE (which may not contain what we expect after being suffixified). I'm thinking we should come up with a more direct way of representing field names, instead of doing the code-gen each time and expecting the hashes to be the same.

What if we had a special term for it?
Or included the field names in the metadata like we want to do for term local variables?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Developer experience improvements or workflow fixes E1 Lowest effort/time I2 Medium impact pretty-printer R3 High reach
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants