Skip to content

Commit

Permalink
Revert "✨ Expose pager heuristic as part of API"
Browse files Browse the repository at this point in the history
This reverts commit d5419fe.
  • Loading branch information
bash committed Jan 24, 2025
1 parent d5419fe commit 2f6a7b2
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 79 deletions.
35 changes: 14 additions & 21 deletions crates/pycolorsaurus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,49 +33,42 @@ create_exception!(colorsaurus, ColorsaurusError, PyException);

/// Detects if the terminal is dark or light.
#[pyfunction]
#[pyo3(signature = (*, timeout=None, require_terminal_on_stdout=false))]
fn color_scheme(
timeout: Option<Timeout>,
require_terminal_on_stdout: bool,
) -> PyResult<ColorScheme> {
imp::color_scheme(query_options(timeout, require_terminal_on_stdout))
#[pyo3(signature = (*, timeout=None))]
fn color_scheme(timeout: Option<Timeout>) -> PyResult<ColorScheme> {
imp::color_scheme(query_options(timeout))
.map(ColorScheme::from)
.map_err(to_py_error)
}

/// Queries the terminal for it's foreground and background color.
#[pyfunction]
#[pyo3(signature = (*, timeout=None, require_terminal_on_stdout=false))]
fn color_palette(
timeout: Option<Timeout>,
require_terminal_on_stdout: bool,
) -> PyResult<ColorPalette> {
imp::color_palette(query_options(timeout, require_terminal_on_stdout))
#[pyo3(signature = (*, timeout=None))]
fn color_palette(timeout: Option<Timeout>) -> PyResult<ColorPalette> {
imp::color_palette(query_options(timeout))
.map(ColorPalette)
.map_err(to_py_error)
}

/// Queries the terminal for it's foreground color.
#[pyfunction]
#[pyo3(signature = (*, timeout=None, require_terminal_on_stdout=false))]
fn foreground_color(timeout: Option<Timeout>, require_terminal_on_stdout: bool) -> PyResult<Color> {
imp::foreground_color(query_options(timeout, require_terminal_on_stdout))
#[pyo3(signature = (*, timeout=None))]
fn foreground_color(timeout: Option<Timeout>) -> PyResult<Color> {
imp::foreground_color(query_options(timeout))
.map(Color)
.map_err(to_py_error)
}

/// Queries the terminal for it's background color.
#[pyfunction]
#[pyo3(signature = (*, timeout=None, require_terminal_on_stdout=false))]
fn background_color(timeout: Option<Timeout>, require_terminal_on_stdout: bool) -> PyResult<Color> {
imp::background_color(query_options(timeout, require_terminal_on_stdout))
#[pyo3(signature = (*, timeout=None))]
fn background_color(timeout: Option<Timeout>) -> PyResult<Color> {
imp::background_color(query_options(timeout))
.map(Color)
.map_err(to_py_error)
}

fn query_options(timeout: Option<Timeout>, require_terminal_on_stdout: bool) -> imp::QueryOptions {
let mut options =
imp::QueryOptions::default().with_require_terminal_on_stdout(require_terminal_on_stdout);
fn query_options(timeout: Option<Timeout>) -> imp::QueryOptions {
let mut options = imp::QueryOptions::default();
options.timeout = timeout.map(|t| t.0).unwrap_or(options.timeout);
options
}
Expand Down
11 changes: 0 additions & 11 deletions crates/terminal-colorsaurus/doc/caveats.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,3 @@ the terminal with another program. This might be the case
if you expect your output to be used with a pager e.g. `your_program` | `less`.
In that case, a race condition exists because the pager will also set the terminal to raw mode.
The `pager` example shows a heuristic to deal with this issue.

If you expect your output to be on stdout then you should enable [`QueryOptions::require_terminal_on_stdout`]:

```rust,no_run
use terminal_colorsaurus::{color_palette, QueryOptions, color_scheme};
let options = QueryOptions::default().with_require_terminal_on_stdout(true);
let theme = color_scheme(options).unwrap();
```

See the `pager` example for more details.
12 changes: 10 additions & 2 deletions crates/terminal-colorsaurus/examples/pager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,19 @@
//! 3. `cargo run --example pager | cat`—should not print the color scheme. This is a false negatives.
//! 4. `cargo run --example pager 2>&1 >/dev/tty | less`—should print the color scheme (or error). This is a false positive.
use std::io::{stdout, IsTerminal as _};
use terminal_colorsaurus::{color_palette, Error, QueryOptions};

fn main() -> Result<(), display::DisplayAsDebug<Error>> {
let options = QueryOptions::default().with_require_terminal_on_stdout(true);
eprintln!("Here's the color palette: {:#?}", color_palette(options)?);
if stdout().is_terminal() {
eprintln!(
"Here's the color scheme: {:#?}",
color_palette(QueryOptions::default())?
);
} else {
eprintln!("No color scheme for you today :/");
}

Ok(())
}

Expand Down
21 changes: 2 additions & 19 deletions crates/terminal-colorsaurus/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::fmt::CaretNotation;
use core::fmt;
use std::time::Duration;
use std::{error, fmt, io};
use std::{error, io};

/// An error returned by this library.
#[derive(Debug)]
Expand All @@ -14,10 +15,6 @@ pub enum Error {
/// either the terminal does not support querying for colors \
/// or the terminal has a lot of latency (e.g. when connected via SSH).
Timeout(Duration),
/// Stdout is not connected to a terminal, but [`QueryOptions::require_terminal_on_stdout`] was set.
///
/// [`QueryOptions::require_terminal_on_stdout`]: `crate::QueryOptions::require_terminal_on_stdout`
NotATerminal(NotATerminalError),
/// The terminal does not support querying for the foreground or background color.
UnsupportedTerminal,
}
Expand All @@ -26,7 +23,6 @@ impl error::Error for Error {
fn source(&self) -> Option<&(dyn error::Error + 'static)> {
match self {
Error::Io(source) => Some(source),
Error::NotATerminal(source) => Some(source),
_ => None,
}
}
Expand All @@ -46,7 +42,6 @@ impl fmt::Display for Error {
Error::Timeout(timeout) => {
write!(f, "operation did not complete within {timeout:?}")
}
Error::NotATerminal(e) => fmt::Display::fmt(e, f),
Error::UnsupportedTerminal {} => {
write!(f, "the terminal does not support querying for its colors")
}
Expand All @@ -59,15 +54,3 @@ impl From<io::Error> for Error {
Error::Io(source)
}
}

#[derive(Debug)]
#[non_exhaustive]
pub struct NotATerminalError;

impl error::Error for NotATerminalError {}

impl fmt::Display for NotATerminalError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "stdout is not connected to a terminal")
}
}
14 changes: 0 additions & 14 deletions crates/terminal-colorsaurus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,26 +143,12 @@ pub struct QueryOptions {
///
/// See [Feature Detection](`feature_detection`) for details on how this works.
pub timeout: std::time::Duration,

/// Only query the terminal for its colors if stdout is a terminal.
///
/// This is used to heuristically avoid race-conditions with pagers.
pub require_terminal_on_stdout: bool,
}

impl QueryOptions {
/// Sets [`Self::require_terminal_on_stdout`].
pub fn with_require_terminal_on_stdout(mut self, require_terminal_on_stdout: bool) -> Self {
self.require_terminal_on_stdout = require_terminal_on_stdout;
self
}
}

impl Default for QueryOptions {
fn default() -> Self {
Self {
timeout: std::time::Duration::from_secs(1),
require_terminal_on_stdout: false,
}
}
}
Expand Down
13 changes: 1 addition & 12 deletions crates/terminal-colorsaurus/src/xterm.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::error::NotATerminalError;
use crate::io::{read_until2, TermReader};
use crate::quirks::{terminal_quirks_from_env, TerminalQuirks};
use crate::{Color, ColorPalette, Error, QueryOptions, Result};
use std::io::{self, BufRead, BufReader, IsTerminal, Write as _};
use std::io::{self, BufRead, BufReader, Write as _};
use std::time::Duration;
use terminal_trx::{terminal, RawModeGuard};

Expand Down Expand Up @@ -102,8 +101,6 @@ fn query<T>(
write_query: impl FnOnce(&mut dyn io::Write) -> io::Result<()>,
read_response: impl FnOnce(&mut Reader<'_>) -> Result<T>,
) -> Result<T> {
ensure_is_terminal(options.require_terminal_on_stdout)?;

if quirks.is_known_unsupported() {
return Err(Error::UnsupportedTerminal);
}
Expand All @@ -127,14 +124,6 @@ fn query<T>(
Ok(response)
}

fn ensure_is_terminal(require_terminal_on_stdout: bool) -> Result<()> {
if require_terminal_on_stdout && !io::stdout().is_terminal() {
Err(Error::NotATerminal(NotATerminalError))
} else {
Ok(())
}
}

fn read_color_response(r: &mut Reader<'_>) -> Result<Vec<u8>> {
let mut buf = Vec::new();
r.read_until(ESC, &mut buf)?; // Both responses start with ESC
Expand Down

0 comments on commit 2f6a7b2

Please sign in to comment.