Use shell variables to configure the builtin.

Instead of the `-s K=V` options, configuration is now read from shell variables:

  TIMEHISTORY_FORMAT sets the default format format, if `-f` is not given.
  TIMEHISTORY_LIMIT sets the maximum history size.

TIMEHISTORY_LIMIT is a dynamic variable, so it is always in sync with the value
stored in the History instance.

README.md

@@ -189,7 +189,8 @@ with a function like this in the `~/.bashrc` file:
 ```bash
 _load_timehistory() {
     enable -f /usr/lib/bash/libtimehistory_bash.so timehistory
-    # use 'timehistory -s' to change configuration, if needed
+    # TIMEHISTORY_LIMIT=…
+    # TIMEHISTORY_FORMAT='…'
 }
 ```
 
@@ -207,8 +208,8 @@ automatically.
 Type `timehistory` to see all entries in the history list.
 
 Every entry is rendered using the default [format string]. To use a different
-[format string] you can change the configuration setting with `-s format='…'`,
-or add the `-f '…'` option in the command-line.
+[format string] you can use the `$TIMEHISTORY_FORMAT` shell variable, or add
+the `-f '…'` option in the command-line.
 
 To see a single entry, type `timehistory <n>`, where `<n>` is the number of the
 entry. If the number starts with a plus sign (`+`), the number is relative to
@@ -251,7 +252,7 @@ Type `timehistory --help` or `help timehistory` to see all available options:
 
 ```console
 $ timehistory --help
-timehistory: timehistory [-f FMT | -v | -j] [<n> | +<n>] | -s | -s SET | -R
+timehistory: timehistory [-f FMT | -v | -j] [<n> | +<n>] | -s | -R
     Displays information about the resources used by programs executed in
     the running shell.
 
@@ -260,7 +261,7 @@ timehistory: timehistory [-f FMT | -v | -j] [<n> | +<n>] | -s | -s SET | -R
                 instead of the default value.
       -v        Use the verbose format, similar to GNU time.
       -j        Print information as JSON format.
-      -s SET    Change the value of a setting. See below.
+      -s        Print the current configuration settings.
       -R        Remove all entries in the history.
 
     If <n> is given, it displays information for a specific history entry.
@@ -272,28 +273,23 @@ timehistory: timehistory [-f FMT | -v | -j] [<n> | +<n>] | -s | -s SET | -R
       Use '-f help' to get information about the formatting syntax.
 
     Settings:
-      The following settings are available:
+      The following shell variables can be used to change the configuration:
 
-        format  Default format string.
-        limit   History limit.
-
-      To change a setting, use '-s name=value', where 'name' is any of the
-      previous values. Use one '-s' for every setting to change.
-
-      '-s' with no argument shows the current settings.
+        TIMEHISTORY_FORMAT      Default format string.
+        TIMEHISTORY_LIMIT       History limit.
 ```
 
 ## Configuration
 
-timehistory accepts two configuration settings:
+timehistory configuration can be modified using shell variables:
 
-* `format`
+* `TIMEHISTORY_FORMAT`
 
     Set the default [format string] for history entries.
 
     This value is used when the timehistory is invoked without the `-f` option.
 
-* `limit`
+* `TIMEHISTORY_LIMIT`
 
     Set the maximum number of entries stored in the history list.
 
@@ -304,24 +300,8 @@ The current configuration settings are printed with `timehistory -s`:
 
 ```console
 $ timehistory -s
-format = [header,table]%n\t%(time:%X)\t%P\t%e\t%C
-limit  = 100
+TIMEHISTORY_FORMAT = [header,table]%n\t%(time:%X)\t%P\t%e\t%C
+TIMEHISTORY_LIMIT  = 100
 ```
 
-To change a setting, use `-s name=value`.
-
-```console
-$ timehistory -s format='%p' -s limit=10
-
-$ timehistory -s
-format = %p
-limit  = 10
-
-$ timehistory -s limit=500
-
-$ timehistory -s
-format = %p
-limit  = 500
-````
-
 [format string]: ./FORMAT.md

src/history.rs

@@ -1,10 +1,12 @@
 //! Command history.
 
 use std::collections::VecDeque;
-use std::ffi::OsString;
+use std::ffi::{CStr, CString, OsString};
+use std::io::{self, Write};
 use std::sync::Mutex;
 use std::time::Duration;
 
+use bash_builtins::variables::DynamicVariable;
 use chrono::{DateTime, Local, TimeZone};
 use once_cell::sync::Lazy;
 
@@ -138,3 +140,32 @@ impl History {
         };
     }
 }
+
+/// Dynamic variable to control the history limit.
+pub struct LimitVariable;
+
+impl DynamicVariable for LimitVariable {
+    fn get(&mut self) -> std::option::Option<CString> {
+        let size = match crate::history::HISTORY.try_lock() {
+            Ok(h) => h.size,
+            Err(_) => return None,
+        };
+
+        CString::new(size.to_string()).ok()
+    }
+
+    fn set(&mut self, value: &CStr) {
+        let size = match value.to_str().map(str::parse) {
+            Ok(Ok(n)) => n,
+
+            _ => {
+                let _ = writeln!(io::stderr(), "timehistory: invalid number");
+                return;
+            }
+        };
+
+        if let Ok(mut history) = crate::history::HISTORY.try_lock() {
+            history.set_size(size);
+        }
+    }
+}

src/lib.rs

@@ -1,13 +1,15 @@
 //! timehistory bash builtin
 
-use bash_builtins::{builtin_metadata, Args, Builtin, BuiltinOptions};
+use bash_builtins::{builtin_metadata, variables, warning, Args, Builtin, BuiltinOptions};
 use bash_builtins::{Error::Usage, Result as BuiltinResult};
+
+use std::borrow::Cow;
 use std::io::{self, BufWriter, Write};
 
 builtin_metadata!(
     name = "timehistory",
     try_create = TimeHistory::new,
-    short_doc = "timehistory [-f FMT | -v | -j] [<n> | +<n>] | -s | -s SET | -R",
+    short_doc = "timehistory [-f FMT | -v | -j] [<n> | +<n>] | -s | -R",
     long_doc = "
         Displays information about the resources used by programs executed in
         the running shell.
@@ -17,7 +19,7 @@ builtin_metadata!(
                 \tinstead of the default value.
           -v\tUse the verbose format, similar to GNU time.
           -j\tPrint information as JSON format.
-          -s SET\tChange the value of a setting. See below.
+          -s\tPrint the current configuration settings.
           -R\tRemove all entries in the history.
 
         If <n> is given, it displays information for a specific history entry.
@@ -29,15 +31,10 @@ builtin_metadata!(
           Use '-f help' to get information about the formatting syntax.
 
         Settings:
-          The following settings are available:
-
-            format\tDefault format string.
-            limit\tHistory limit.
-
-          To change a setting, use '-s name=value', where 'name' is any of the
-          previous values. Use one '-s' for every setting to change.
+          The following shell variables can be used to change the configuration:
 
-          '-s' with no argument shows the current settings.
+            TIMEHISTORY_FORMAT\tDefault format string.
+            TIMEHISTORY_LIMIT\tHistory limit.
     ",
 );
 
@@ -55,10 +52,13 @@ use std::time::Duration;
 
 const DEFAULT_FORMAT: &str = "[header,table]%n\\t%(time:%X)\\t%P\\t%e\\t%C";
 
-struct TimeHistory {
-    /// Default format to print history entries.
-    default_format: String,
-}
+/// Shell variable to set the format string.
+const SHELL_VAR_FORMAT: &str = "TIMEHISTORY_FORMAT";
+
+/// Shell variable to set the history limit.
+const SHELL_VAR_LIMIT: &str = "TIMEHISTORY_LIMIT";
+
+struct TimeHistory;
 
 #[derive(BuiltinOptions)]
 enum Opt<'a> {
@@ -100,15 +100,15 @@ impl TimeHistory {
             return Err("shared buffer unavailable".into());
         }
 
+        variables::bind(SHELL_VAR_LIMIT, history::LimitVariable)?;
+
         procs::replace_functions()?;
 
         unsafe {
             history::OWNER_PID = libc::getpid();
         }
 
-        Ok(TimeHistory {
-            default_format: DEFAULT_FORMAT.into(),
-        })
+        Ok(TimeHistory)
     }
 }
 
@@ -161,18 +161,16 @@ impl Builtin for TimeHistory {
                 }
 
                 Opt::Setting(Some(setting)) => {
+                    warning!("-s is deprecated. Use the shell variables to change the settings");
+
                     let mut parts = setting.splitn(2, '=');
                     match (parts.next(), parts.next()) {
                         (Some("limit"), Some(value)) => {
                             history.set_size(value.parse()?);
                         }
 
                         (Some("format"), Some(value)) => {
-                            self.default_format = if value.is_empty() {
-                                DEFAULT_FORMAT.into()
-                            } else {
-                                value.to_owned()
-                            };
+                            variables::set(SHELL_VAR_FORMAT, value)?;
                         }
 
                         (Some(name), _) => {
@@ -215,13 +213,13 @@ impl Builtin for TimeHistory {
         args.finished()?;
 
         let format = match &output_format {
-            None => Some(self.default_format.as_ref()),
-            Some(Output::Format(f)) => Some(f.as_ref()),
-            Some(Output::Verbose) => Some(include_str!("format/verbose.fmt")),
+            None => Some(Self::default_format()),
+            Some(Output::Format(f)) => Some(Cow::Borrowed(f.as_ref())),
+            Some(Output::Verbose) => Some(include_str!("format/verbose.fmt").into()),
             Some(Output::Json) => None,
         };
 
-        let format = format.map(format::FormatOptions::parse);
+        let format = format.as_deref().map(format::FormatOptions::parse);
 
         // Render output as a table.
         if let Some(options) = &format {
@@ -286,13 +284,19 @@ impl TimeHistory {
         write!(
             &mut output,
             "\
-             format = {}\n\
-             limit  = {}\n\
+             TIMEHISTORY_FORMAT = {}\n\
+             TIMEHISTORY_LIMIT  = {}\n\
             ",
-            self.default_format,
+            Self::default_format(),
             history.size(),
         )?;
 
         Ok(())
     }
+
+    fn default_format() -> Cow<'static, str> {
+        variables::find_as_string(SHELL_VAR_FORMAT)
+            .and_then(|s| s.into_string().ok().map(Cow::Owned))
+            .unwrap_or_else(|| DEFAULT_FORMAT.into())
+    }
 }

src/tests/shell/change-config.test.sh

@@ -3,13 +3,14 @@
 
 load_builtin
 
-timehistory -s limit=5000 -s format='%n\t%P\t%C'
+TIMEHISTORY_LIMIT=5000
+TIMEHISTORY_FORMAT='%n\t%P\t%C'
 
 ASSERT_OUTPUT \
   "timehistory -s" \
   <<-'ITEMS'
-	format = %n\t%P\t%C
-	limit  = 5000
+	TIMEHISTORY_FORMAT = %n\t%P\t%C
+	TIMEHISTORY_LIMIT  = 5000
 ITEMS
 
 timehistory -s format='> %C'
@@ -18,3 +19,17 @@ command expr 1 + 2
 ASSERT_OUTPUT \
   "timehistory" \
   "> expr 1 '+' 2"
+
+
+# Backward compatibility.
+timehistory -s limit=123 -s format='%N\t%P'
+ASSERT_OUTPUT \
+  'echo "${TIMEHISTORY_FORMAT:-NA} ${TIMEHISTORY_LIMIT:-NA}"' \
+  '%N\t%P 123'
+
+
+# Check variables after deleting the builtin.
+enable -d timehistory
+ASSERT_OUTPUT \
+  'echo "${TIMEHISTORY_FORMAT:-NA} ${TIMEHISTORY_LIMIT:-NA}"' \
+  '%N\t%P NA'