purescript-language-server icon indicating copy to clipboard operation
purescript-language-server copied to clipboard

Add command for generating an optic for a type

Open JordanMartinez opened this issue 3 years ago • 5 comments

I've created purescript-tidy-codgen-lens, which can generate a Lens.purs file for each type declaration in a file, as well as the record label lenses. However, this program works on entire files.

Could part of this program be extracted into a plugin, so that if I highlight a given type, and run the 'Generate Optic' command, it would generate an optic just for that type?

JordanMartinez avatar Jan 03 '22 19:01 JordanMartinez

Seems like a candidate for a code lens (no pun intended). Source selection/running commands is something that requires client-side support so for refactoring/codegen type actions something that can be provided via LSP directly (code lens or info-type code action) should be preferred.

nwolverson avatar Jan 04 '22 15:01 nwolverson

Ok, so what are the first steps towards implementing this?

JordanMartinez avatar Jan 04 '22 18:01 JordanMartinez

I've looked through this codebase a bit. Seems like I'd need to add a file at src/LanguageServer/IdePurescript/CodeLens/OpticsGeneration.purs

A few questions I'll need to address when implementing this:

  • should the labels for record types have their lenses generated? If so, what options should be exposed for configuring where they go?
  • while the code lens is nice for files with only a few declarations, it is likely that we'll want to codegen the declarations for an entire file at times. So, besides the code lens, a code action doing this for the entire module would likely need to be supported. And, similar to the prior question, what options should be exposed for configuring that?
  • in purescript-tidy-codegen-lenses, I'm printing the optic declarations using purescript-tidy-codegen. However, this codebase should be agnostic when it comes to the formatter one uses. How should printing the declarations be resolved?
  • the generated optics will only work if the imports are appropriately updated. How should that be handled? Is the optic itself only generated and the user must import those types separately? Or do we update the imports as well, which may complicate this implementation and the resulting imports might still be invalid?

JordanMartinez avatar Jan 05 '22 00:01 JordanMartinez

In general - at a glance it looks like purescript-tidy-codgen-lens is using the CST parser, which should integrate well here, the parsed input should already be available in the state for a lens to find the appropriate candidate & work from.

I don't really have an opinion on options/1st 2 points, other than concern about potential explosion of LSP config just for 1 thing. If being slightly opinionated cuts down on config that always seems like a good tradeoff to me.

On formatting, my initial take is however it is printed, the user can run their formatter over this as they wish. If purescript-tidy-codegen is the easiest way to construct the output, that seems like a good place to start.

Imports, we do a whole bunch of import management already, it would be nice if this could just work - but it doesn't seem worth worth worrying about up front

nwolverson avatar Jan 06 '22 11:01 nwolverson

I think the initial version of this should only generate a lens or prism for the type in question and not care about record labels. That will likely solve 80% of the problem most people have. The rest is other niceties that can be added after the fact.

JordanMartinez avatar Jan 14 '22 02:01 JordanMartinez