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

Editable typedefs: deletion #401

Closed
georgefst opened this issue Apr 19, 2022 · 3 comments · Fixed by #1153
Closed

Editable typedefs: deletion #401

georgefst opened this issue Apr 19, 2022 · 3 comments · Fixed by #1153

Comments

@georgefst
Copy link
Contributor

From "not in spec" section of #267:

The deletion of parts of a type definition is harder to handle, since we want to avoid the student losing code that they've written. In addition to the operations outlined above, the following are also possible:

  • Delete type
  • Delete constructor
  • Delete parameter from constructor
  • Delete parameter from type
    • Concerns about losing code are somewhat lesser here in practice, but a student could in theory have an arbitrarily-complex type-level expression in a no-longer-required type application.
    • Note that we can only allow this when the parameter does not appear in any constructors. Or at least, we should remove the parameter from any constructors automatically as a first step.

We should consider these alongside other changes which can cause code to disappear. See Saving the student's work when a type changes.

@brprice
Copy link
Contributor

brprice commented Aug 15, 2023

@georgefst Could you update us on the status of this?

IIUC, we support deletions of "unused" types/constructors/parameters-of-ctors/parameters-of-types?

@georgefst
Copy link
Contributor Author

Yes, we now support deleting parts of unused typedefs. This was implemented in #1063 and got a bit lost in the Zurihac maelstrom.

The check is not as fine-grained as it could be (e.g. we should be able to delete an unused constructor even when its type is in use - #1063 (comment)).

Anyway, we're now intending to just warn about more destructive deletions, instead of banning them. See #1133.

@georgefst
Copy link
Contributor Author

georgefst commented Sep 28, 2023

I consider #1153 to close this issue, since all that now remains is tracked in #1133 and #402.

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

Successfully merging a pull request may close this issue.

3 participants