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

Support presentaion mode by haskell-mode-show-type-at #598

Merged
merged 4 commits into from
May 10, 2015
Merged

Support presentaion mode by haskell-mode-show-type-at #598

merged 4 commits into from
May 10, 2015

Conversation

geraldus
Copy link
Contributor

Make haskell-mode-show-type-at function aware of presentation mode.

Refactored code a bit: bind multiple occurances of
(haskell-fontify-as-mode ty 'haskell-mode) to variable fty.

@geraldus
Copy link
Contributor Author

@chrisdone asked me to make it optional

If so make it optional; I originally had types using presentation mode, but in the end I found it too heavy-weight to just see the type.

If I understood him correctly he talked about possibility to have an option not to use presentation mode when displaying type, even if haskell-process-use-presentaion-mode is set to t.

Documentation of haskell-process-use-presentaion-mode says:

Use presentation mode to show things like type info instead of
printing to the message area.

So, this variable is a kind of global option, i.e. use presentation mode for everything. The only idea I have to optionally disable presentation mode for arbitrary functions is to define some variables and make presentation mode checks slightly smarter (hence complex).

So I need some feedback and guidance how to deal with this.

@geraldus
Copy link
Contributor Author

BTW, I used same style as in haskell-process-do-info for naming of presentation buffer, e.g *Haskell Presentation:type subject* (in case of do-info it looks like *Haskell Presentation:info subject*). As you can see, there is no whitespace before colons, what about adding it to separate Haskell Presentation and :info parts, e.g. *Haskell Presentation:info … and *Haskell Presentation:type …?

@gracjan
Copy link
Contributor

gracjan commented Apr 25, 2015

Hmm, can we do a proper cleanup while we are at touching presentation mode?

As far as I see haskell-mode-show-type-at calls haskell-mode-type-at to get the type which executes :type-at %s %d %d %d %d %s in current ghci-ng session.

This is async-pretending-to-be-sync. As example of other way of doing similar thing lets look at haskell-process-do-info. That function calls haskell-process-do-simple-echo that in turn calls haskell-process-queue-command and its :complete handler does if haskell-process-use-presentation-mode.

I think haskell-mode-show-type-at should be rewritten in terms of haskell-process-do-simple-echo. And then we get presentation mode for free. Can you check if this is doable?

@geraldus
Copy link
Contributor Author

Surely, in fact to implement this functionally I've used haskell-process-do-info and haskell-process-do-simple-echo as examples to understand how support of presentation mode is implemented in haskell-process-do-info. I'll update this PR when will be ready.

@geraldus
Copy link
Contributor Author

I don't understand well things like :go, :complete, :state etc. inside functions bodies. Where can I read about it? @chrisdone, I was suggested to ask you about it. Can you help a bit?

@geraldus
Copy link
Contributor Author

@gracjan, it turns that haskell-mode-show-type-at takes optional parameter which indicates is it necessary to insert type signature ahead expression or not, e.g. if we call this function with t passed as argument (or hit C-u C-c C-t) having point at expression xs = mapM_ print we'll end up with something like this

xs :: [a] -> IO ()
xs = mapM_ print

As you already mentioned it is blocking operation. I assume it will be good to make haskell-mode-show-type-at completely async. It this case we can define another function to get type information and insert resulting type signature, e.g. haskell-mode-sync-insert-type-signature and replace all use cases of (haskell-mode-show-type-at t) with it.

@geraldus
Copy link
Contributor Author

Here my new implementation. I've implemented two new functions:

haskell-mode-async-show-type-at
haskell-mode-sync-insert-type-signature

Now, if haskell-mode-show-type-at is used without parameter it calls async function, and in other case — blocking sync function. This will provide backward compatibility. BTW, from time to time inserting inferred type signature hangs Emacs for ages.

Small notice: haskell-process-do-simple-echo takes optional MODE argument. It goes into :state

(make-haskell-command
  :state (list process line mode)
  …)

haskell-process-do-info passes 'haskell-mode as MODE, so did I, but have no strong opinion what is its purpose.

@geraldus
Copy link
Contributor Author

One caveat: surprisingly, I forgot to show type information when haskell-mode-show-type-at is called with parameter, but is it still needed to be shown? In this case type signature will be inserted in buffer and user will see it anyway.

@gracjan
Copy link
Contributor

gracjan commented Apr 26, 2015

Good stuff! Especially the async version!

I could merge this as is, but we can improve this even further, shall we?

How about this:

Lets add a flag saying please-insert-type-signature-in-the-buffer to the :state of async call. Then when the call ends it will insert the type signature provided nothing has changed in the buffer.

How to detect if nothing has changed in the buffer? That is a bit tricky, basically it is 1) set a flag saying that nothing has changed so far 2) add post-command-hook that clears that flag. Check flag as needed.

So the full algorithm:

  1. Use the async version for everything, pass the insert-value to :state.
  2. Create a flag somewhere, add a post-command-hook.
  3. Let the async call run in asyncronously.
  4. When it finishes it should check the insert-value and nothing-has-changed flags. If so it should insert the result in the buffer as does the current sync version.

Note: that everything will be async and will never render Emacs unusable. Any user command will have the function of cancel in this context.

Note also that similar functionality can be used for other synchronous commands.

What do you think?

@geraldus
Copy link
Contributor Author

One more question: there is also such functions as haskell-process-insert-type, and haskell-process-do-type, they works only for identifiers but not for region. I suppose this is just special cases of haskell-show-type-at function, isn't it? If so, we could change them to simply exploit show-type-at function, right?
Or maybe completely get rid of them.

Update: that functions use another REPL command — :type, while show-type-at uses :type-at which have different semantics.

@geraldus
Copy link
Contributor Author

geraldus commented May 5, 2015

@gracjan original function is haskell-mode-show-type-at, it is still untouched (async and sync version if type insertion is needed) and you need to test temporal function haskell-mode-totally-async-show-type-at (temporarily bound to C-c C-y); please look at *Messages* buffer: I need to know what commands are reported there.

@geraldus
Copy link
Contributor Author

geraldus commented May 5, 2015

I tried to put (setq this-command …) to functions:

  • haskell-process-queue-command
  • haskell-process-cmd-queue-add
  • haskell-process-send-string

and them does not appear in last-command.

Also, I've added messages before adding hook, which makes obvious that last-command and real-last-command are not updated.

@gracjan
Copy link
Contributor

gracjan commented May 6, 2015

Hmm, you should not be setting this-command, rather read from it. See this:

Watching changes... 
For now real last command is `aquamacs-right-char' 
For now last command is `aquamacs-right-char'
[HOOK] Post-command hook triggered
real last command is `aquamacs-right-char'
last command is `aquamacs-right-char'
this command is `haskell-mode-totally-async-show-type-at'
Watch job finished.

Then I added this:

   ;; wait 5 sec
   (format "Control.Concurrent.threadDelay 5000000\n:type-at %s %d %d %d %d %s"

And then I got:

Inferring Control.Concurrent.threadDelay 5000000
:type-at /Users/gracjan/Sources/haskell-mode/Main.hs 3 1 3 5 main…
Watching changes...
For now real last command is `previous-line'
For now last command is `previous-line'
[HOOK] Post-command hook triggered
real last command is `previous-line'
last command is `previous-line'
this command is `haskell-mode-totally-async-show-type-at'
[HOOK] Post-command hook triggered
real last command is `haskell-mode-totally-async-show-type-at'
last command is `haskell-mode-totally-async-show-type-at'
this command is `aquamacs-right-char'
[HOOK] Post-command hook triggered
real last command is `aquamacs-right-char'
last command is `aquamacs-right-char'
this command is `aquamacs-right-char'
[HOOK] Post-command hook triggered
real last command is `aquamacs-right-char'
last command is `aquamacs-right-char'
this command is `aquamacs-right-char'
Restarting server
[HOOK] Post-command hook triggered
real last command is `aquamacs-right-char'
last command is `aquamacs-right-char'
this command is `next-line'
Watch job finished.

So everything look super good to me. Am I mistaken? If post-command-hook is called more than once ( > 1) then 'something has possibly changed' and insert type should be canceled. I would not depend on the value of this-command as this sounds fishy.

(Note: one more thing to sort out is when C-c C-y C-c C-y is executed in quick succession. As I see it will try to establish two watchers and then call haskell-process-async-stop-watching-changes twice and that will conflict, won't it?).

@gracjan
Copy link
Contributor

gracjan commented May 6, 2015

@geraldus: :type is available in normal GHCi, :type-at requires GHCi-ng. It would be super-unfriendly, especially to newbies, to require special GHCi.

@geraldus
Copy link
Contributor Author

geraldus commented May 6, 2015

@gracjan, as for :type-at command, I know about GHCi-ng dependency, but this was in the original function. Also, the difference between these commands, is that :type-at is able to successfully returns type signatures of exressions in localised let statements, that's why you pass to it actual bounds of exression; and :type command will not be albe to infer type of x in following example:

y :: IO ()
y = let x = mapM_ print in x [1, 2, 3]

@geraldus
Copy link
Contributor Author

geraldus commented May 6, 2015

@gracjan huh! cool trick with threadDelay call, I wouldn't never guessed myself. Well, so we can just count hooks, but we also can keep this as list, and output something like

These commands prevented type signature insertion: right-chat, next line...

And what about several command launced simultaneously: we can make our flag to be a map (like in haskell, I believe this is alist in Elisp) and share key through :state.

@geraldus
Copy link
Contributor Author

geraldus commented May 7, 2015

@gracjan, ok, I was managed to introduce flag as alist, then I switched to plist in sake of speed; finally I've noticed that commands are executed in sequence, one by one (surprise! this is haskell-process-queue-command), so there will never be a case when two watchers conflict, because each watch job starts inside :go section.

@geraldus
Copy link
Contributor Author

geraldus commented May 7, 2015

@gracjan I've uploaded pre-final version of code. Almost everything works fine, but there is issue with regions: original (sync) version of function will wrap region with parentheses and append to expression inferred type, e.g. mapM_ print becomes (mapM_ print :: forall a. Show a => [a] -> IO ()), but async version always looses region when signature arrives and inserts it above expression, though in both cases it uses exactly same function to insert type signature: haskell-process-insert-type-signature.

Also, I need to test how things work when process returns error message.

@geraldus
Copy link
Contributor Author

geraldus commented May 9, 2015

@gracjan please review code and especially doc strings.
BTW, https://github.com/geraldus/haskell-mode/commit/427622080d5f7232f36415737e1ad79e24a323a9 makes a lot warnings visible (from emacs-lisp-checkdoc checker).

I'm going to have 4 commits in the end:

  1. define helper functions such as echo-or-present, insert-type-signature
  2. define changes-flag related stuff
  3. core functionality
  4. commentary and footer warnings fix

+ hs-utils/capture-expr-bounds
+ hs-utils/compose-type-at-command
+ hs-utils/reduce-string
+ hs-utils/insert-type-signature
+ hs-utils/echo-or-present
@gracjan
Copy link
Contributor

gracjan commented May 9, 2015

Issue: eval (haskell-mode-show-type-at t) inserts value AND opens presentation. It should only insert value.

@gracjan gracjan mentioned this pull request May 9, 2015
6 tasks
geraldus added 3 commits May 10, 2015 12:39
Defined buffer local flag `hs-utils/async-post-command-flag`

Defined flag related functions:

+ hs-utils/async-update-post-command-flag
+ hs-utils/async-watch-changes
+ hs-utils/async-stop-watching-changes
Make function asyncronous, remove unnecessary synchronous
`haskell-mode-type-at` function.

Insert type signature only if nothing changed and there was valid
response.

Present result in case of presentation mode, otherwise put it in echo
area.

Do not present result if asked to insert result.
gracjan added a commit that referenced this pull request May 10, 2015
Support presentaion mode by `haskell-mode-show-type-at`
@gracjan gracjan merged commit 2532074 into haskell:master May 10, 2015
@geraldus geraldus deleted the present-type branch May 10, 2015 08:01
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 this pull request may close these issues.

2 participants