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

Quiet mode #29

Open
roboguy13 opened this issue Aug 5, 2015 · 15 comments
Open

Quiet mode #29

roboguy13 opened this issue Aug 5, 2015 · 15 comments

Comments

@roboguy13
Copy link
Member

It would be nice to have a command line switch that makes hermit-shell act more like hermit, by not displaying the result of every command run when it executes a script. When hermit runs a script, it only displays the state at the very end, after it runs every command.

Maybe the quiet mode should be on by default in hermit-shell too.

It looks like this is the line where it is being printed. Maybe we should wrap the existing Shell type in a Reader containing these kinds of options.

@roboguy13
Copy link
Member Author

Well, I figured out a way to do this with a zero parameter type class and I added it to a new branch with this commit. None of the things that print stuff out is being imported directly by the hermit-shell executable source. It is being imported by the stuff that is loaded in by hermit-shell. As a result, we can define a type class

class ShellSettings where
  quietMode :: Bool

and use it like this

send :: ShellSettings => Shell a -> IO a
send (Shell g) = do
<...>
    case ... of
         <...>
         ShellResult gss a
          | quietMode -> return a
          | otherwise -> do
                 sequence_ [ withNoStyle sty txt
                           | gs <- gss
                           , Glyph txt sty <- gs
                           ]
                 putStrLn "\n[Done]\n"
                 return a

The trick is: we don't define an instance in any of the normal source files. Instead, we define it in the .ghci-hermit file depending on whether or not we get the --quiet command line option.

We could add to the ShellSettings type class to have global settings for other things like colorized output as well.

Another nice thing about this is that GHCi lets you redefine instances whenever you want, so we could change the settings without too much trouble (at least, we could define .ghci : commands that should be able to).

What does everyone think? Ugly hack or a reasonable solution?

@roboguy13
Copy link
Member Author

At some point, we should add something that tells the server not to try to send anything back at all if quiet mode is enabled.

@xich
Copy link
Member

xich commented Aug 20, 2015

I'd give that 'reasonable hack' status... seems nice and neat to me, though it means you cannot modify the settings at runtime like you could with state/reader. Is that a problem?

This paper might even worth reading:

http://dl.acm.org/citation.cfm?id=1017481

I seem to remember it does something similar.

@roboguy13
Copy link
Member Author

@xich That's the paper I had in mind, ha. At first, I was thinking I would need reflection for it to work at all.

Also actually, I just edited my comment above with regard to that. It depends on where you want to modify it. GHCi lets you redefine instances whenever you want (at any point using the REPL, you can enter instance ShellSettings where quietMode = ... and it behaves how you'd hope), so we should be able to define a GHCi command to switch it on or off (I'm not sure if you could make a single command that toggles it though).

If we need more than that, we'll probably need reflection using a technique like in that paper.

@RyanGlScott
Copy link
Member

I like that zero-parameter typeclass idea a lot better than my old HERMIT.Debughack. We should probably replace all uses of when debug with ShellSettings.

@ecaustin
Copy link
Contributor

Is there a use case where we would want to trigger "quiet mode" on/off programmatically instead of interactively, e.g. in a script or test suite?

Also, at what point do we abandon the behemoth that our .ghci file is becoming and write a proper wrapper executable?
This may be a good impetus for doing so, as if we are adding multiple configuration flags we probably want a help flag to explain what they do, right?

@roboguy13
Copy link
Member Author

@ecaustin I'm not sure if we would want to change it inside a script, but with a test suite we always have control over the arguments we give to the hermit-shell command so that shouldn't be an issue.

The --help flag is already in place and it is separate from the .ghci file things (which reminds me, I should add info to it in this branch).

By a wrapper executable, do you mean something which would send everything we have as .ghci commands to the main hermit-shell program on initialization or something which would intercept all the REPL input, check for special commands and then hand it off to the main program?

The second one would probably be pretty tricky because we (I think) need to have a GHCi session continuously running so would might need some interprocess communication. If that's the case, I would also wonder how the Windows compatibility would work out.

(Also, I figured out a hack to let us toggle quiet mode on and off with a :quiet command, but I'm not sure if it'll scale well yet)

@roboguy13
Copy link
Member Author

Oh, it looks like it actually relies on something in the hermit package for printing --help. Either way, we should probably change that.

@roboguy13
Copy link
Member Author

Evan and I discussed this yesterday and I agree with his suggestion of using a global IORef. We will always use this inside IO so we don't need to worry about using tricks to maintain purity.

@ecaustin
Copy link
Contributor

I'm reopening this for continued discussion because I think there was some confusion about the comments I made.
At no point in time did I endorse the use of unsafePerformIO.

My suggestion was to initialize a reference in the main thread of the program and pass it around either explicitly or implicitly using a monad.
Now that I've seen the changes you've made, I see that we can't do that because the output you want to suppress is triggered by our overloaded GHCI pretty-printer, not a method in our main execution loop.

I agree that the output behavior of the old shell was preferable, so I've removed your changes and redefined printForRepl to use void for Shell commands. If people want the added output then they can make an explicit call to send and GHCI will handle the rest, provided that the result type has a Show instance.

I've simultaneously switched send to print response glyphs with their attached style, so we should have identical output as the old shell.
Obviously we'll need to regenerate the golden output for tests now.

If we want to make the behavior of printForRepl configurable then I have a few ideas that will keep us away from hacky workarounds, but I think the current comprise is close to, if not exactly, what we want.

By the way, this is in no way an indictment of you or the code you wrote.
I just really, really hate unsafePerformIO.

@ecaustin ecaustin reopened this Aug 25, 2015
@roboguy13
Copy link
Member Author

@ecaustin Haha, that's understandable. I'm not a big fan of unsafePerformIO either (in fact, this is one of the only times I've ever used it). I misunderstood your suggestion.

It looks like it still prints all the output though, currently. Is that the expected behavior for the moment?

@ecaustin
Copy link
Contributor

Now it's my turn to have misunderstood things.
I thought the goal was to remove the duplicate output from the interactive shell.

If we want to silence the intermediate output from running scripts then we should transmit them in their entirety instead of step by step, such that the client only gets the final result.

I believe this corresponds to @andygill's "deep embedding" version of the remote monad protocol.

@roboguy13
Copy link
Member Author

@ecaustin Oh, I see. There's been another miscommunication. I was intending quiet mode to be an optional switch to turn off intermediate output from scripts (which is the original HERMIT's default setting).

Also I'm not positive, but I was under the impression that the end goal was to do the deep embedding style thing where we send everything over at the start. @andygill would know for sure.

It seems to me that both modes could be useful: seeing intermediate steps could help with debugging scripts and, on the other hand, you might not want to have to print everything when you know that a transformation is working (or if you only want to take a look at the result of the last step without the overhead of printing every other step).

@xich
Copy link
Member

xich commented Aug 25, 2015

I think what @ecaustin is getting at is this:

  • Currently, lines in a script are sent over one-by-one. Hence, you get the resulting AST printed for each intermediate step.

  • Instead of having a switch to suppress AST printing for certain commands (quiet mode), just send over the entire script at once, so only the final AST is printed.

  • The 'stopScript' command should control this batching... so:

    foo
    bar
    stopScript
    baz
    

    gets sent over as foo ; bar and then the AST resulting after bar is returned.

@ecaustin
Copy link
Contributor

Yup, @xich nailed it on the head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants