atom-haskell / ide-haskell-repl

GHCi REPL in Atom
https://atom.io/packages/ide-haskell-repl
MIT License
26 stars 8 forks source link

Repl inputs not being written to its history #62

Closed freeman42x closed 6 years ago

freeman42x commented 6 years ago

This happens on v0.7.13 also.

How to reproduce:

module Ch13 where

import Control.Monad
import System.IO (hSetBuffering, stdout, BufferMode(..))
import System.Exit (exitSuccess)
import Data.Char (isLetter, toLower)

-- Madam I’m Adam,

palindrome :: IO ()
palindrome = do
  hSetBuffering stdout NoBuffering
  forever $ do
    line <- getLine
    if isPalindrome line
      then do
        putStrLn "It's a palindrome!"
        exitSuccess
      else putStrLn "Nope!"

isPalindrome :: String -> Bool
isPalindrome s = sm == reverse sm
  where
    sm = toLower <$> filter isLetter s

Load the module in the repl with :l Type palindrome and Type anything. Whatever you typed will not be written to the REPL history. If the Interrupts current computation button is pressed then the strings typed will appear in the history.

Expected behavior (repl run from terminal):

*Ch13> palindrome qwe Nope! nqwejbqwe Nope! Madam I'm Adam It's a palindrome! *** Exception: ExitSuccess

Extra note, when interrupting, in the Repl tab it is printed:

Interrupted. Variable not in scope: quehquehiquehiwe

So it appears that the Repl was treating the palindrome program input as input to the Repl itself and there was no variable named quehquehiquehiwe

This works as intended if run directly from GHCI.

Ref: https://github.com/atom-haskell/ide-haskell-repl/issues/61

lierdakil commented 6 years ago

I thought I explained caveats and limitations well enough, but here's a gif

idehaskellrepl62

freeman42x commented 6 years ago

@lierdakil I did read the caveats and limitations. In my test I did press enter to send a newline before pressing shift + enter and it still did not work. I don't get how it is working on your machine.

lierdakil commented 6 years ago

Are you on Windows by any chance? If so, could you run a quick test?

  1. Open REPL
  2. Run palindrome or whatever, getLine will do.
  3. Open developer console (View → Developer → Toggle Developer Tools → Console tab or something similar)
  4. Paste and run the following snippet:
    document.querySelector('atom-text-editor.ide-haskell-repl').getModel().setText('asd\r\n'); atom.commands.dispatch(document.querySelector('atom-text-editor.ide-haskell-repl'), 'ide-haskell-repl:exec-command');
  5. See if asd shows up in repl output
freeman42x commented 6 years ago

I am on NixOS, Linux.

Following the instructions asd appears in Repl and Nope! after it.

freeman42x commented 6 years ago

Getting quite a few warnings from when Atom is started in the dev console:

IntersectionObserver.observe(target): target element is not a descendant of root. TreeView @ :121025 /home/neo/.atom/packages/haskell-ghc-mod/lib/util.js:43 haskell-ghc-mod warning: Reading cabal sandbox config failed with Error: ENOENT: no such file or directory, open '/home/neo/HaskellLearning/IdeTest/cabal.sandbox.config' warn @ /home/neo/.atom/packages/haskell-ghc-mod/lib/util.js:43 /home/neo/.atom/packages/haskell-ghc-mod/lib/util.js:43 haskell-ghc-mod warning: No cabal sandbox found warn @ /home/neo/.atom/packages/haskell-ghc-mod/lib/util.js:43 4/home/neo/.atom/packages/haskell-ghc-mod/lib/util.js:43 haskell-ghc-mod warning: Warning: 'dist/setup-config' exists, ignoring Stack project

on the commandline: warning: -package-conf=/home/neo/.ghc-mod/cabal-helper/Cabal-2.0.0.2-db-8.0.2 is deprecated: Use -package-db instead [1 of 5] Compiling CabalHelper.Types ( CabalHelper/Types.hs, /home/neo/.ghc-mod/cabal-helper/CabalHelper/Types.o ) [2 of 5] Compiling CabalHelper.Sandbox ( CabalHelper/Sandbox.hs, /home/neo/.ghc-mod/cabal-helper/CabalHelper/Sandbox.o ) [3 of 5] Compiling CabalHelper.Licenses ( CabalHelper/Licenses.hs, /home/neo/.ghc-mod/cabal-helper/CabalHelper/Licenses.o )

CabalHelper/Licenses.hs:56:8: error: • Expecting one more argument to ‘CPackageIndex ModuleName’ Expected a type, but ‘CPackageIndex ModuleName’ has kind ‘ -> ’ • In the type signature: findTransitiveDependencies :: CPackageIndex ModuleName -> Set CInstalledPackageId -> Set CInstalledPackageId

on the commandline: warning: -package-conf=/home/neo/.ghc-mod/cabal-helper/Cabal-2.0.0.2-db-8.0.2 is deprecated: Use -package-db instead [1 of 5] Compiling CabalHelper.Types ( CabalHelper/Types.hs, /home/neo/.ghc-mod/cabal-helper/CabalHelper/Types.o ) [2 of 5] Compiling CabalHelper.Sandbox ( CabalHelper/Sandbox.hs, /home/neo/.ghc-mod/cabal-helper/CabalHelper/Sandbox.o ) [3 of 5] Compiling CabalHelper.Licenses ( CabalHelper/Licenses.hs, /home/neo/.ghc-mod/cabal-helper/CabalHelper/Licenses.o )

CabalHelper/Licenses.hs:56:8: error: • Expecting one more argument to ‘CPackageIndex ModuleName’ Expected a type, but ‘CPackageIndex ModuleName’ has kind ‘ -> ’ • In the type signature: findTransitiveDependencies :: CPackageIndex ModuleName -> Set CInstalledPackageId -> Set CInstalledPackageId EXCEPTION: browse: readCreateProcess: /nix/store/07wnp40jsxzavj5n31ld2n1536w87spa-cabal-helper-0.7.3.0/libexec/x86_64-linux-ghc-8.0.2/cabal-helper-0.7.3.0/cabal-helper-wrapper "--with-ghc=ghc" "--with-ghc-pkg=ghc-pkg" "--with-cabal=cabal" "/home/neo/HaskellLearning/IdeTest" "/home/neo/HaskellLearning/IdeTest/dist" "package-db-stack" "entrypoints" "source-dirs" "ghc-options" "ghc-src-options" "ghc-pkg-options" "ghc-merged-pkg-options" "ghc-lang-options" "licenses" "flags" "config-flags" "non-default-config-flags" "compiler-version" (exit 1): failed

warn @ /home/neo/.atom/packages/haskell-ghc-mod/lib/util.js:43

freeman42x commented 6 years ago

But not getting any errors/warnings when doing palindrome test with extra new line for inputs. Not even after pressing interrupt and the inputs get shown.

lierdakil commented 6 years ago

Well, ghc-mod is apparently broken because it can't handle Cabal 2. Also a good chance your ghc package database is broken since you don't use sandboxes apparently (run ghc-pkg check to check for broken packages, haddock warnings are safe to ignore). These warnings are not relevant to the problem at hand though.

I'm pretty sure getLine on Linux only looks for LF (\n) and doesn't care about CR (\r).

Can you run the same test but use this snippet instead:

document.querySelector('atom-text-editor.ide-haskell-repl').getModel().setText('asd\n'); atom.commands.dispatch(document.querySelector('atom-text-editor.ide-haskell-repl'), 'ide-haskell-repl:exec-command');

?

freeman42x commented 6 years ago

[neo@nixos:~/HaskellLearning/IdeTest]$ ghc-pkg check Warning: haddock-interfaces: /nix/store/rykisz7fl7lyaqzbhp9n0fdh57aqdf9v-fail-4.9.0.0/share/doc/x86_64-linux-ghc-8.0.2/fail-4.9.0.0/html/fail.haddock doesn't exist or isn't a file Warning: haddock-html: /nix/store/rykisz7fl7lyaqzbhp9n0fdh57aqdf9v-fail-4.9.0.0/share/doc/x86_64-linux-ghc-8.0.2/fail-4.9.0.0/html doesn't exist or isn't a directory Warning: haddock-interfaces: /nix/store/jvl5i3pffs56r3k9w0wan9sh3pnwjmxy-either-4.4.1.1/share/doc/x86_64-linux-ghc-8.0.2/either-4.4.1.1/html/either.haddock doesn't exist or isn't a file Warning: haddock-html: /nix/store/jvl5i3pffs56r3k9w0wan9sh3pnwjmxy-either-4.4.1.1/share/doc/x86_64-linux-ghc-8.0.2/either-4.4.1.1/html doesn't exist or isn't a directory

I deleted the dist folder since I'm actually using stack for the build.

freeman42x commented 6 years ago

I run the test with the new snippet and it also prints asd in the repl and Nope!

lierdakil commented 6 years ago

Which is actually really strange, because the second snippet is equivalent to:

  1. Removing all text from ide-haskell-repl input box
  2. Writing asd
  3. Pressing 'enter'
  4. Pressing 'shift+enter'

modulo redefined keybindings

lierdakil commented 6 years ago

Any updates? Is this still relevant?

freeman42x commented 6 years ago

Tested with 0.7.15, the issue is still happening.

I think this is relevant since it changes GHCI behavior in a commonly used case.

lierdakil commented 6 years ago

My point is: it's entirely unclear why. I can not reproduce this on my end, and when running controlled tests via code, the issue doesn't manifest on your end either. So... if you could provide any insight, that would be nice. Otherwise, I don't know what I could possibly do about this.

freeman42x commented 6 years ago

I tracked down the issue to: https://github.com/atom-haskell/ide-haskell-repl/blob/master/src/views/ide-haskell-repl-view.tsx#L93

When the test is run via command line this.ghci.isBusy() returns true and the stdin program input is printed correctly.

When run by typing manually asa the value of this.ghci.isBusy() is false.

lierdakil commented 6 years ago

Ah, of course, now it makes sense. Thank you! I'll try to cook up a fix in a bit.

freeman42x commented 6 years ago

@lierdakil Awesome, thank you! Quite curious to see what the issue was. I've been investigating for quite some time.

lierdakil commented 6 years ago

The root of the issue is the package attempts to send autocompletion requests for stdin input. Which is obviously pointless, but what's worse, messes up isBusy logic.

And the reason why I couldn't reproduce on my end is that I have autocomplete-plus option 'Show Suggestions On Keystroke' disabled.

freeman42x commented 6 years ago

Cool, I get it now. Still weird that no one else run into this issue yet. Was starting to believe that it might be something related to my setup / OS.

lierdakil commented 6 years ago

Still weird that no one else run into this issue yet

Not many people use ide-haskell-repl (when compared to ide-haskell itself), and I imagine even fewer use stdin for input. I personally prefer passing string arguments to functions instead of fumbling with getLine -- for one, most functions don't live in IO in the first place.

On a brighter note, v0.7.16 is up :tada:, which should fix this issue (and a few other potential race conditions similar to this one). Let me know if it works for you.

I also added a small busy indicator near prompt while at it: image (in the upper left corner)

Again, thanks for your help!

freeman42x commented 6 years ago

Thank you! It works fine on the latest version.