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

[Feature Request] Add missing fields in record #333

Closed
GuillaumeDesforges opened this issue Aug 17, 2020 · 9 comments
Closed

[Feature Request] Add missing fields in record #333

GuillaumeDesforges opened this issue Aug 17, 2020 · 9 comments
Labels
type: enhancement New feature or request

Comments

@GuillaumeDesforges
Copy link

GuillaumeDesforges commented Aug 17, 2020

It would be great if HLS could hint a code action to add (and import) missing fields of a record.

Say I write (using the process library)

import System.Process (CreateProcess(CreateProcess), CmdSpec(RawCommand))

proc = CreateProcess { cmdspec = RawCommand "echo" [ "hello world" ] }

many other fields are missing (see doc), and need to be imported.

@pepeiborra
Copy link
Collaborator

I think this should be a completion provider

@ndmitchell
Copy link
Collaborator

A completion provider would be great. But in Rust, it's also a diagnostic fix, which has been a very nice way to use it.

@gdevanla
Copy link
Contributor

gdevanla commented Sep 9, 2020

I plan to take a stab at this, thanks to @pepeiborra's awesome tutorial on how to write a plugin.

So, should I start exploring this as a completion provider? I was able to explore the Example.hs, so far to build a sample code snippet so that each field in the record could correspond to a placeholder.

Can one of you provide me pointers, on where I can explore the AST information in ghcide for this?

@pepeiborra
Copy link
Collaborator

Look at how completions are done in ghcide, in particular check https://github.com/haskell/ghcide/blob/9ae5134d79972405415ce5062dbae67aeb938f21/src/Development/IDE/Plugin/Completions/Logic.hs#L232

You will need to generate new completions for records that include not only the constructor, but also the fields. For that you will need to find all the TyThings that correspond to constructors fo record types and figure out the names of all the fields.

@gdevanla
Copy link
Contributor

gdevanla commented Sep 14, 2020

I am testing this now, and we do get completions for records. I have added some tests to show what I found so far:
haskell/ghcide#804

Looks, like what we need is inspection of currently used fields in the context and provide a code action for missing fields. Which kind of suggests, we should use diagnostics as the original issue comment suggested.

Now, I need to look at, how to determine if I am inside a context of record creation, inspect the currently declared fields and identify the missing fields. Some scenarios, where this will not be correct is when def is used. At that point a code-action provided in that context may not be accurate.

Of course, in terms of enhancements, creating a code-snippet, as soon as we see a CiConstructor would also be nice. That could be a different issue/task.

@gdevanla
Copy link
Contributor

gdevanla commented Sep 20, 2020

I observed and added tests to show that missing record fields are already being report under diagnostics by hls. haskell/ghcide#824.

Please suggest if we would like to extend this any further as part of this issue. Perhaps, we need a code action generated for each missing field?

@gdevanla
Copy link
Contributor

I have started iterating over the changes required for this feature. For now, I have support for Local completions. Please could I have some feedback on the initial approach and if I can proceed in this path to support completions that are available from other modules. #512

pepeiborra added a commit that referenced this issue Dec 27, 2020
Before this change:
[nix-shell:~/scratch/ghcide]$ /home/pepe/scratch/ghcide/dist-newstyle/build/x86_64-linux/ghc-8.8.1/ghcide-0.0.6/x/ghcide/build/ghcide/ghcide  +RTS --info
 [("GHC RTS", "YES")
 ,("GHC version", "8.8.1")
 ,("RTS way", "rts_thr_p")
 ,("Build platform", "x86_64-unknown-linux")
 ,("Build architecture", "x86_64")
 ,("Build OS", "linux")
 ,("Build vendor", "unknown")
 ,("Host platform", "x86_64-unknown-linux")
 ,("Host architecture", "x86_64")
 ,("Host OS", "linux")
 ,("Host vendor", "unknown")
 ,("Target platform", "x86_64-unknown-linux")
 ,("Target architecture", "x86_64")
 ,("Target OS", "linux")
 ,("Target vendor", "unknown")
 ,("Word size", "64")
 ,("Compiler unregisterised", "NO")
 ,("Tables next to code", "YES")
 ,("Flag -with-rtsopts", "-A128M")
 ]

After this change:
[nix-shell:~/scratch/ghcide]$ /home/pepe/scratch/ghcide/dist-newstyle/build/x86_64-linux/ghc-8.8.1/ghcide-0.0.6/x/ghcide/build/ghcide/ghcide  +RTS --info
 [("GHC RTS", "YES")
 ,("GHC version", "8.8.1")
 ,("RTS way", "rts_thr")
 ,("Build platform", "x86_64-unknown-linux")
 ,("Build architecture", "x86_64")
 ,("Build OS", "linux")
 ,("Build vendor", "unknown")
 ,("Host platform", "x86_64-unknown-linux")
 ,("Host architecture", "x86_64")
 ,("Host OS", "linux")
 ,("Host vendor", "unknown")
 ,("Target platform", "x86_64-unknown-linux")
 ,("Target architecture", "x86_64")
 ,("Target OS", "linux")
 ,("Target vendor", "unknown")
 ,("Word size", "64")
 ,("Compiler unregisterised", "NO")
 ,("Tables next to code", "YES")
 ,("Flag -with-rtsopts", "-I0 -qg -A128M")
 ]
@gdevanla
Copy link
Contributor

Can we close this issue now?

@jneira jneira added the status: needs info Not actionable, because there's missing information label Jan 14, 2021
@jneira
Copy link
Member

jneira commented Jan 14, 2021

Yeah, thanks @gdevanla

@jneira jneira closed this as completed Jan 14, 2021
@jneira jneira removed the status: needs info Not actionable, because there's missing information label Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants