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

Interactive mode #28

Closed
Bodigrim opened this issue Oct 14, 2022 · 4 comments · Fixed by #29
Closed

Interactive mode #28

Bodigrim opened this issue Oct 14, 2022 · 4 comments · Fixed by #29
Labels
💡 idea enhancement New feature or request

Comments

@Bodigrim
Copy link
Contributor

I'd like to add an interactive mode, which combines watch and profile without an intermediate file, streaming Cabal output into plotter. Here is a demo (sorry for a large GIF):

render1665770208930

@chshersh chshersh added enhancement New feature or request 💡 idea labels Oct 15, 2022
@chshersh
Copy link
Owner

@Bodigrim Wow, that looks amazing! 🤩
I'd be happy to see this in dr-cabal 👏🏻

I believe, you already implemented this and this is not just a drawn GIF? I had problems in the past with the getTerminalSize function that can't get the terminal size when I redirect output to dr-cabal. It always returned Nothing here not matter what I tried 😞

terminalWidth <- getTerminalSize >>= \case
Just (_height, width) -> pure width
Nothing -> do

@Bodigrim
Copy link
Contributor Author

System.Console.ANSI.getTerminalSize calls setCursorPosition h 9999 9999 and then retrieves getCursorPosition to find the actual terminal size. This trickery does not work at all when stdin is not a terminal. One can use https://hackage.haskell.org/package/terminal-size to measure terminal window reliably.

Yes, I have a very crude, but working implementation in https://github.com/Bodigrim/dr-cabal/commit/c483233cc4065daf2e3d52c306abccda0343e47c

@chshersh
Copy link
Owner

@Bodigrim Thanks for the explanation! I see. So terminal-size looks indeed like a better option here 👍🏻
I'm happy to switch to it.

Feel free to open a PR with your implementation when you're happy with it 👍🏻

One suggestion I could make: I would make interactive the default behaviour of the watch command. One of the reasons I have two separate commands watch and profile is that I wasn't able to get the terminal size on watching.

If it's possible, I imagine the following ideal API:

  • By default, the dr-cabal profile command watches the output and prints interactive profiling output on any changes.
  • You have the --non-interactive flag to disable this behaviour and simply redirect cabal output and print the result in the end only once
  • The profile command has the --to-file | --from-file option to control whether it should save the output to the file or just print the result from the file
  • The watch command is no longer needed

I'm not sure about the best way to proceed here. I'm not releasing dr-cabal now but I'd love to release and announce a new version after the end of Hacktoberfest since you made lots of cool improvements! But I'd like to make this command refactoring before the next version. So you can implement interactive mode now as a separate command and then I can create a separate issue to refactor CLI and somebody (maybe even me) will do it.

@Bodigrim
Copy link
Contributor Author

So you can implement interactive mode now as a separate command and then I can create a separate issue to refactor CLI and somebody (maybe even me) will do it.

Sounds good to me, I'm not really up to refactoring CLI right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 idea enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants