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

Write the implicit cradle to disk #837

Closed
pepeiborra opened this issue Sep 13, 2020 · 12 comments
Closed

Write the implicit cradle to disk #837

pepeiborra opened this issue Sep 13, 2020 · 12 comments

Comments

@pepeiborra
Copy link
Collaborator

I think this is great and I'm happy to merge it, but I have a couple of requests that can either be done in this PR of in follow-ups:

  1. Inform the user that there is no cradle haskell/ghcide#788
  2. Write the implicit cradle to disk
  3. Give the user a choice if there are both stack and cabal project descriptors
  4. Delete all the cradle files from ghcide and hls - no longer needed

Originally posted by @pepeiborra in haskell/ghcide#782 (comment)

@ndmitchell
Copy link
Collaborator

Does this implicit cradle stuff flow into HLS when we upgrade ghcide for free? Or is this something we need to duplicate there?

@fendor
Copy link
Collaborator

fendor commented Sep 13, 2020

What should happen if there is already a hie.yaml file? Just not generate?

Also, regarding generation, e.g. for recent cabal versions:

cradle:
  cabal:

should be sufficient. Should we think about a backwards compatibility story?

@pepeiborra
Copy link
Collaborator Author

Does this implicit cradle stuff flow into HLS when we upgrade ghcide for free? Or is this something we need to duplicate there?

Flows for free

@pepeiborra
Copy link
Collaborator Author

What should happen if there is already a hie.yaml file? Just not generate?

Then we are not in the implicit case

Also, regarding generation, e.g. for recent cabal versions:


cradle:

  cabal:

should be sufficient. Should we think about a backwards compatibility story?

That's implicit-hie responsibility. I don't have a view

@jneira
Copy link
Member

jneira commented Sep 13, 2020

My hope was that the implicit cradle were good enough to not write the file and to remove the need of hie.yaml itself.
Only memory, mostly transparent for the user.
But, well, in the actual context, this makes sense so 👍

EDIT: I still hope that someday we will be able to not make users write the build info twice (or even make the tool write for them a second one on disk)

@Avi-D-coder
Copy link
Collaborator

Avi-D-coder commented Sep 13, 2020

My original intent was to write a implicit-hie.yaml.

The issue with this approach was invalidating a previously generated config.

I believe what we had talked about over at the hls repo was a lsp action to generate a config, rather than always writing the config out. That way of you use common stanzas or cabal conditionals you could have a place to start from without us having to worry about invalidation.

I am not against always generating writing a config to disk, but the auto generated file could not be named hie.yaml.

@pepeiborra
Copy link
Collaborator Author

I don't follow. The implicit case only arises when there exists no cradle in disk. Therefore there is no risk of invalidating anything. The generated file could have a header "# Autogenerated by gen-hie on 01-01-2001" and instructions on how to regenerate it.

@Avi-D-coder
Copy link
Collaborator

Avi-D-coder commented Sep 14, 2020

Let's say you add a component to your project. If you used gen-hie > hie.yaml, you need to manually rerun the command.

If we want to handle cradle generation automatically in a noob friendly way we need to handle changes in component structure. implicit-hie-cradle handles this by generating a hie-bios Cradle from scratch every time. We could write a hie.yaml with comment containing the hash of the cabal file it was generated from. Then regenerate the hie.yaml when the cabal file changes, but there are a number of edge cases with this approach. Hence Implicit-hie.yaml, so we don't clobber a hie.yaml, that the user modified, and neglected to delete the hash comment line. Implicit-hie.yaml would be overwritten before every session to avoid falling out of date.

I think writing the config to the LSP log is a better option, but unconditionally generating a lower presence yaml config could also work.

Sparsely parsing the cabal file and generating Cradles as Implicit-hie does is almost free. I see a big advantage in showing the auto generated cradles in yaml form. It could even be useful to log an auto generated config when a hie.yaml exists, to show any missed components. But writing hie.yaml is what gen-hie, does and I intended gen-hie as a stopgap, until a more automatic solution could make it into the LSP server.

@jneira
Copy link
Member

jneira commented Sep 14, 2020

Agree with @Avi-D-coder, write the main config file without user control has its own caveats:

  • Maybe i dont want to write the hie.yaml in most cases, cause the implicit cradle works for those cases. And hopefully it will improve until it will cover all cases
  • You write it once and the implicit cradle is not triggered until you manually delete the hie.yaml. It will be not auto up to date when changing components/source dirs, like the previous implicit cradle, so some users can be disturbed.

So to have an alternative config file until we have a code action/config option to generate the file on explicit user demand seems to be a better option.

@pepeiborra
Copy link
Collaborator Author

I see, so you want to make hie.yaml optional. Fair enough, that's a great end state if it works.

No need to write the implicit cradle to disk then. Assuming it always does the right thing, it's an unnecessary ugliness and best not recorded in the file system.

@jneira
Copy link
Member

jneira commented Nov 16, 2020

@pepeiborra we are mostly happily using implicit-hie-cradle, would be some of the follow-ups still needed?

Maybe

@pepeiborra
Copy link
Collaborator Author

Happy to close this task and open new ones for follow-ups when/if reported

@pepeiborra pepeiborra transferred this issue from haskell/ghcide Jan 1, 2021
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

No branches or pull requests

5 participants