From a24c39f13f0f8b6bda61ca21bf48de2bd1dce147 Mon Sep 17 00:00:00 2001 From: gesh Date: Wed, 26 Feb 2025 20:27:47 +0200 Subject: [PATCH] fix: Use POSIX API to compute tty width (#2332) On some systems, TERM is set to a value that doesn't have a valid terminfo entry. Rather than hackily fall back on a value for TERM that appears to work in most contexts (TERM=dumb) but which isn't guaranteed anywhere to be valid, use proper POSIX ioctls to get the tty width. This has the added bonus of also working on Windows. In fact, we already settled on computing the terminal size in this way in hledger-lib, so this commit centralizes the choice of the logic there. Also added a note for alternative methods and their tradeoffs, in case this turns out to be fragile on some systems. --- hledger-lib/Hledger/Utils/IO.hs | 22 ++++++++++++++++++++++ hledger/Hledger/Cli/CliOptions.hs | 12 ++---------- hledger/hledger.cabal | 17 ----------------- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/hledger-lib/Hledger/Utils/IO.hs b/hledger-lib/Hledger/Utils/IO.hs index 91b856ffd..857b4df38 100644 --- a/hledger-lib/Hledger/Utils/IO.hs +++ b/hledger-lib/Hledger/Utils/IO.hs @@ -413,8 +413,30 @@ hasOutputFile = do -- Terminal size +-- [NOTE: Alternative methods of getting the terminal size] +-- terminal-size uses the TIOCGWINSZ ioctl to get the window size on Unix +-- systems, which may not be completely portable according to people in +-- #linux@liberachat. +-- +-- If this turns out to be the case, supplementary coverage can be given by +-- using the terminfo package. +-- +-- Conversely, terminfo on its own is not a full solution, firstly because it +-- only works on Unix (not Windows), and secondly since in some scenarios (eg +-- stripped-down build systems) the terminfo database may be limited and lack +-- the correct entries. (A hack that sometimes works but which isn't robust +-- enough to be relied upon is to set TERM=dumb -- while this advice does appear +-- in some places, it's not guaranteed to work) +-- +-- In any case, $LINES/$COLUMNS should not be used as a source for the terminal +-- size -- that is a bashism, and in particular they aren't updated as the +-- terminal is resized. +-- +-- See #2332 for details + -- | An alternative to ansi-terminal's getTerminalSize, based on -- the more robust-looking terminal-size package. +-- -- Tries to get stdout's terminal's current height and width. getTerminalHeightWidth :: IO (Maybe (Int,Int)) getTerminalHeightWidth = fmap (fmap unwindow) size diff --git a/hledger/Hledger/Cli/CliOptions.hs b/hledger/Hledger/Cli/CliOptions.hs index 8efbf0edc..1b1997f47 100644 --- a/hledger/Hledger/Cli/CliOptions.hs +++ b/hledger/Hledger/Cli/CliOptions.hs @@ -107,9 +107,7 @@ import String.ANSI import System.Console.CmdArgs hiding (Default,def) import System.Console.CmdArgs.Explicit import System.Console.CmdArgs.Text -#ifndef mingw32_HOST_OS -import System.Console.Terminfo -#endif +import Hledger.Utils.IO (getTerminalWidth) import System.Directory import System.Environment import System.Exit (exitSuccess) @@ -618,13 +616,7 @@ rawOptsToCliOpts rawopts = do let iopts = rawOptsToInputOpts day usecolor postingaccttags rawopts rspec <- either error' pure $ rawOptsToReportSpec day usecolor rawopts -- PARTIAL: mcolumns <- readMay <$> getEnvSafe "COLUMNS" - mtermwidth <- -#ifdef mingw32_HOST_OS - return Nothing -#else - (`getCapability` termColumns) <$> setupTermFromEnv - -- XXX Throws a SetupTermError if the terminfo database could not be read, should catch -#endif + mtermwidth <- getTerminalWidth let availablewidth = NE.head $ NE.fromList $ catMaybes [mcolumns, mtermwidth, Just defaultWidth] -- PARTIAL: fromList won't fail because non-null list return defcliopts { rawopts_ = rawopts diff --git a/hledger/hledger.cabal b/hledger/hledger.cabal index 5d027ec80..eb14485e9 100644 --- a/hledger/hledger.cabal +++ b/hledger/hledger.cabal @@ -98,11 +98,6 @@ flag debug manual: True default: False -flag terminfo - description: On POSIX systems, build with the terminfo lib for detecting terminal width - manual: False - default: True - flag threaded description: Build with support for multithreaded execution manual: False @@ -195,9 +190,6 @@ library , utility-ht >=0.0.13 , wizards >=1.0 default-language: Haskell2010 - if (!(os(windows))) && (flag(terminfo)) - build-depends: - terminfo if (flag(debug)) cpp-options: -DDEBUG @@ -247,9 +239,6 @@ executable hledger , utility-ht >=0.0.13 , wizards >=1.0 default-language: Haskell2010 - if (!(os(windows))) && (flag(terminfo)) - build-depends: - terminfo if (flag(debug)) cpp-options: -DDEBUG if flag(threaded) @@ -300,9 +289,6 @@ test-suite unittest , utility-ht >=0.0.13 , wizards >=1.0 default-language: Haskell2010 - if (!(os(windows))) && (flag(terminfo)) - build-depends: - terminfo if (flag(debug)) cpp-options: -DDEBUG @@ -353,8 +339,5 @@ benchmark bench , wizards >=1.0 buildable: False default-language: Haskell2010 - if (!(os(windows))) && (flag(terminfo)) - build-depends: - terminfo if (flag(debug)) cpp-options: -DDEBUG