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.

Sign up for GitHub

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

Jump to bottom

wrap to terminal width #249

Closed
wants to merge 1 commit into from
Closed

wrap to terminal width #249

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions src/Pretty.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module Pretty (Doc, Pretty(..), string, text, viaShow, (<+>), (<>), align, hang,
import Control.Lens hiding (List)
import Control.Monad.State
import qualified Data.HashMap.Strict as HM
import Data.Maybe (fromMaybe)
import qualified Util.Set as Set
import Prettyprinter hiding (Pretty(..), angles, parens)
import qualified Prettyprinter as PP
Expand All @@ -19,7 +20,10 @@ import Data.Sequence (Seq)
import Data.Text (Text)
import qualified Data.Text as T
import qualified Data.Foldable as F
import System.Environment (lookupEnv)
import System.FilePath (takeFileName)
import System.IO.Unsafe (unsafePerformIO)
import Text.Read (readMaybe)
Comment on lines +23 to +26
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review the new imports for environment handling and unsafe IO operations.

The addition of System.Environment and System.IO.Unsafe is necessary for the new functionality to dynamically fetch the terminal width. However, the use of unsafePerformIO should be carefully justified as it can lead to non-deterministic behavior in a purely functional setting.

Consider alternatives to unsafePerformIO if possible, to maintain the purity of the functions.


import Binding
import Binding.Info
Expand Down Expand Up @@ -58,8 +62,21 @@ angles doc = text "⟨" <> align (group doc) <> "⟩"
vec :: Doc ann -> Doc ann
vec doc = text "[" <> align (group doc) <> "]"

customLayoutOptions :: LayoutOptions
customLayoutOptions = unsafePerformIO $ do
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using unsafePerformIO is the most straightforward way to do it, but of course I expect that somebody will object and that I will have to thread a LayoutOptions variable everywhere.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One concern is that the tests use pretty, so they might start failing depending on the terminal width. But I tried making my terminal very small and the tests still passed 🤷

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's because it doesn't work. echo $COLUMNS prints the right value in the terminal, but lookupEnv "COLUMNS" returns Nothing!

columnsMaybeString <- lookupEnv "COLUMNS"
let columnsMaybeInt :: Maybe Int
columnsMaybeInt = do
str <- columnsMaybeString
readMaybe str
let columns :: Int
columns = fromMaybe 80 columnsMaybeInt

pure $ defaultLayoutOptions
{layoutPageWidth = AvailablePerLine columns 1.0}

Comment on lines +65 to +77
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider refactoring customLayoutOptions to avoid unsafePerformIO.

The function uses unsafePerformIO to fetch the terminal width, which can lead to unpredictable behavior in Haskell. It's generally recommended to keep IO operations explicit in the function's type signature to maintain purity and predictability.

Consider refactoring the function to return IO LayoutOptions instead, and handle the IO operation in the calling context. This change would make the IO operations explicit and maintain the purity of the function.

customLayoutOptions :: IO LayoutOptions
customLayoutOptions = do
  columnsMaybeString <- lookupEnv "COLUMNS"
  let columnsMaybeInt = readMaybe =<< columnsMaybeString
  let columns = fromMaybe 80 columnsMaybeInt
  pure $ defaultLayoutOptions { layoutPageWidth = AvailablePerLine columns 1.0 }

pretty :: Pretty ann a => a -> Text
pretty x = renderStrict (layoutPretty defaultLayoutOptions (pp Env.empty x))
pretty x = renderStrict (layoutPretty customLayoutOptions (pp Env.empty x))

prettyPrint :: Pretty ann a => a -> IO ()
prettyPrint x = putDoc (pp Env.empty x)
Expand All @@ -69,7 +86,7 @@ prettyPrintLn x = putDoc (pp Env.empty x) >> putStrLn ""

prettyEnv :: Pretty ann a => Env Var v -> a -> Text
prettyEnv env x =
renderStrict (layoutPretty defaultLayoutOptions (pp (fmap (const ()) env) x))
renderStrict (layoutPretty customLayoutOptions (pp (fmap (const ()) env) x))

prettyPrintEnv :: Pretty ann a => Env Var v -> a -> IO ()
prettyPrintEnv env x =
Expand Down Expand Up @@ -201,7 +218,7 @@ class PrettyBinder ann a | a -> ann where
instance PrettyBinder VarInfo a => PrettyBinder VarInfo (TyF a) where
ppBind env t =
let subs = ppBind env <$> t
in (pp env (fst <$> subs), foldMap snd subs)
in (pp env (fst <$> subs), foldMap snd subs)

newtype BinderPair = BinderPair (Ident, Var)

Expand Down
Loading