Add fallbackCompletion to support module completion in ghci :add

haskell / haskeline

A Haskell library for line input in command-line programs.

https://hackage.haskell.org/package/haskeline

BSD 3-Clause "New" or "Revised" License

221 stars 75 forks source link

Add fallbackCompletion to support module completion in ghci :add #91

Closed kmicklas closed 5 years ago

kmicklas commented 6 years ago

GHCi's :add command supports both file paths and module names, and I want to patch it to tab complete both.

The first commit is present in GHC's submodule on master, but not in this repo, despite it being listed as the official upstream for haskeline. I'm not sure what's up with that, so if I'm submitting this patch to the wrong place just let me know.

This change is

judah commented 6 years ago

Thanks for pointing out the missing commit. Best I can tell, it looks like I pushed it associated with a tag for the release 0.7.4.3 but never updated the master branch. I've pushed that commit to master as well.

kmicklas commented 6 years ago

@judah Did I address your comments?

kmicklas commented 5 years ago

@judah Bump.

judah commented 5 years ago

Thank you for the clarification. After considering this change some more, I'm not convinced that we should directly use the length of the "ignored" part. It seems like that's a proxy for what we actually care about, namely, whether the completions did anything useful.

To me, "fallback" means that if there are completions from a, then we use them (and ignore b), but if there are no completions from a then we use what b gives us. For example:

mergeCompletions a b input = do
    aCompletions <- a input
    if null (snd aCompletions)
        then b input
        else aResult

What do you think about those semantics?

Also, I've been turning around in my head what you said about the representation of the unused text, and I think you're right that it's suboptimal. I'm starting to consider making a new major release of Haskeline that would improve this API. (But I don't think it should necessarily block this change.)

judah commented 5 years ago

Ah, taking another look I saw you renamed the function to mergeCompletions , sorry for missing that.

Following up, I have two concerns with the semantics as you've given them (which is why I proposed the other implementation):

There's no guarantee that the unused text from a is compatible with b (as you said, this is an inflexibility in Haskeline's API)
Your function mergeCompletions isn't associative; it's not true in general that mergeCompletions a (mergeCompletions b c) == mergeCompletions (mergeCompletions a b) c. (For example, if a and b return lists of length 1, and c returns a list of length 2.)

kmicklas commented 5 years ago

@judah Yeah, agreed. I've put it back to the simple fallback semantics in your example (and changed the name accordingly).

kmicklas commented 5 years ago

@judah I can't seem to find what causes this repo to get pushed to the GHC mirror at http://git.haskell.org, is that a manual process?

judah commented 5 years ago

Yes, I believe it's manual. Maybe try pinging ghc-devs@haskell.org?

FYI, Haskeline's master has #97 which is an API-breaking change, and another one is coming soon. I've created a new branch haskeline-0.7 which preserves the current API and contains this PR, and should be easier to use with the current ghc master.