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

[#31] Refactor and simplify CLI #32

Merged
merged 2 commits into from
Nov 20, 2022

Conversation

chshersh
Copy link
Owner

@chshersh chshersh commented Nov 15, 2022

Resolves #31

I implemented the changes described in #31 🎉

Peek 2022-11-15 21-10

It was a massive refactoring because I realised that changing CLI simply to make the interactive behaviour the default one is not that simple. As a result, the entire DrCabal.Interactive module was removed and I incorporated its changes to DrCabal.Watch (sorry @Bodigrim).

Now:

  • The interactive mode is the default mode
  • The UI is simplified to the single command
  • There's still an ability to read from files and output to files
    • TODO: This needs more testing, I didn't have the time to test it yet

Now, to run the profiling, you only need to do the following command:

cabal --store-dir=$(mktemp -d) --dependencies-only build all | dr-cabal profile

I haven't updated the documentation yet and I will do it as a part of #30.

The only problem so far is when the profiling output is longer than the screen height, I can't scroll back and the output seems to be mangled in a horrible way:

image

@Bodigrim Did you observe the same behaviour when you implemented the interactive command? Maybe I changed something wrongly during this refactoring 🤔

@chshersh chshersh self-assigned this Nov 15, 2022
@chshersh chshersh added 💡 idea enhancement New feature or request labels Nov 15, 2022
@Bodigrim
Copy link
Contributor

The only problem so far is when the profiling output is longer than the screen height, I can't scroll back and the output seems to be mangled in a horrible way

Yeah, I've seen the same effect, when scrolling upwards an output larger than viewpoint. AFAIU this is a fundamental limitation of the primary screen buffer, unless you switch to an alternate screen buffer as full-screen terminal applications do (htop, etc.).

@chshersh
Copy link
Owner Author

@Bodigrim Do you know if there's an easy way to use an alternative screen buffer (like writing a single line of code)?

I find the current behavior undesirable so I'm thinking about possible solutions, e.g. printing only the first "terminal window height" lines during the interactive mode.

@chshersh
Copy link
Owner Author

chshersh commented Nov 16, 2022

Okay, here is a simple program that starts an alternate terminal screen (inspired by this) and and returns back after it. I can try to use it in dr-cabal to see how it goes:

I haven't found relevant functions in ansi-terminal which is too bad 😞

#!/usr/bin/env cabal
{- cabal:
build-depends:
  , base ^>= 4.16
-}

{-# LANGUAGE NumericUnderscores #-}
{-# LANGUAGE ScopedTypeVariables #-}

module Main where

import Control.Concurrent
import Data.Foldable

main :: IO ()
main = do
    putStrLn "\ESC[?1049h\ESC[H"

    for_ [1 .. 5] $ \(i :: Int) -> do
        putStrLn $ "Going back in: " <> show i
        threadDelay 1_000_000

    putStrLn "\ESC[?1049l"

@chshersh
Copy link
Owner Author

Anyway, here's a relevant issue in ansi-terminal:

@chshersh
Copy link
Owner Author

By default, dr-cabal now outputs the profiling chart in the alternate screen buffer (and then duplicates the final result after switching to the normal buffer).

Peek 2022-11-20 17-20

It handles exceptions and SIGINT correctly if interrupted during interactive output so it would switch back again correctly. Nothing is printed in the terminal on Ctrl+C but this could be one of the future improvements 🙂

There's so much I can do in a limited time... But at least I'm satisfied enough with the current behaviour, although, it took a while to achieve the result 😮‍💨

@chshersh chshersh merged commit 29ea82f into main Nov 20, 2022
@chshersh chshersh deleted the chshersh/31-Refactor-and-simplify-CLI branch November 20, 2022 17:31
@Bodigrim
Copy link
Contributor

Looks great!

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 this pull request may close these issues.

Refactor and simplify CLI
2 participants