Skip to content

Commit

Permalink
bug: Use en_US when fetching EPOCHREALTIME
Browse files Browse the repository at this point in the history 
Isolates fetching of EPOCHREALTIME to a function which sets LC_ALL=en_US.UTF-8.
This ensures that the value is in decimal format, regardless of runtime locale.

bug: Hide duration when no command executed
  • Loading branch information
@davidpfarrell
davidpfarrell committed Oct 30, 2022
1 parent 1c9cfd0 commit 7c7e4f9
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 7 deletions.
21 changes: 18 additions & 3 deletions lib/command_duration.bash
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,24 @@
#
# Functions for measuring and reporting how long a command takes to run.

: "${COMMAND_DURATION_START_SECONDS:=${EPOCHREALTIME:-$SECONDS}}"
# Get shell duration in decimal format regardless of runtime locale.
# Notice: This function runs as a sub-shell - notice '(' vs '{'.
function _shell_duration_en() (
# DFARREL You would think LC_NUMERIC would do it, but not working in my local
LC_ALL='en_US.UTF-8'

This comment has been minimized.

Sign in to view

Copy link
@dayne

dayne Apr 10, 2024

Contributor

On a very recently installed Raspberry Pi I'm finding this sub-shell call causes an error for every new terminal command.

-bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8): No such file or directory

Not optimal. Commenting it out the LC_ALL here makes that error go away with no obvious detrimental effects.

Seems like requiring en_US.UTF-8 to exist isn't a good requirement.. but if it is required then perhaps another check is needed to verify the system you are on has en_US.UTF-8.
locale on a recently installed Raspberry Pi is:

LANG=en_GB.UTF-8
LANGUAGE=
LC_CTYPE="en_GB.UTF-8"
LC_NUMERIC="en_GB.UTF-8"
LC_TIME="en_GB.UTF-8"
LC_COLLATE="en_GB.UTF-8"
LC_MONETARY="en_GB.UTF-8"
LC_MESSAGES="en_GB.UTF-8"
LC_PAPER="en_GB.UTF-8"
LC_NAME="en_GB.UTF-8"
LC_ADDRESS="en_GB.UTF-8"
LC_TELEPHONE="en_GB.UTF-8"
LC_MEASUREMENT="en_GB.UTF-8"
LC_IDENTIFICATION="en_GB.UTF-8"
LC_ALL=

locale -a

C
C.utf8
en_GB.utf8
POSIX

Reconfiguring my pi to use en_US.UTF-8 is possible:
sudo raspi-config nonint do_change_locale en_US.UTF-8

But I feel like that is possibly cheating for all those non en_US people out there that don't want to switch to that. It does solve the problem though.

This comment has been minimized.

Sign in to view

Copy link
@akinomyoga

akinomyoga Apr 10, 2024

Contributor

I think this line should be updated to use LC_ALL=C. The locale C is required by both the C standard and the POSIX standard.

This comment has been minimized.

Sign in to view

Copy link
@cornfeedhobo

cornfeedhobo Apr 11, 2024

Member

This is great point, thanks for bringing it to our attention.

It would be great if we could open a PR with a test that can catch this if it happens again.
printf "%s" "${EPOCHREALTIME:-$SECONDS}"
)

: "${COMMAND_DURATION_START_SECONDS:=$(_shell_duration_en)}"
: "${COMMAND_DURATION_ICON:=🕘}"
: "${COMMAND_DURATION_MIN_SECONDS:=1}"

function _command_duration_pre_exec() {
COMMAND_DURATION_START_SECONDS="${EPOCHREALTIME:-$SECONDS}"
COMMAND_DURATION_START_SECONDS="$(_shell_duration_en)"
}

function _command_duration_pre_cmd() {
COMMAND_DURATION_START_SECONDS=""
}

function _dynamic_clock_icon {
Expand All @@ -20,13 +32,15 @@ function _dynamic_clock_icon {

function _command_duration() {
[[ -n "${BASH_IT_COMMAND_DURATION:-}" ]] || return
[[ -n "${COMMAND_DURATION_START_SECONDS:-}" ]] || return

local command_duration=0 command_start="${COMMAND_DURATION_START_SECONDS:-0}"
local -i minutes=0 seconds=0 deciseconds=0
local -i command_start_seconds="${command_start%.*}"
local -i command_start_deciseconds=$((10#${command_start##*.}))
command_start_deciseconds="${command_start_deciseconds:0:1}"
local current_time="${EPOCHREALTIME:-$SECONDS}"
local current_time
current_time="$(_shell_duration_en)"
local -i current_time_seconds="${current_time%.*}"
local -i current_time_deciseconds="$((10#${current_time##*.}))"
current_time_deciseconds="${current_time_deciseconds:0:1}"
Expand Down Expand Up @@ -59,3 +73,4 @@ function _command_duration() {
}

_bash_it_library_finalize_hook+=("safe_append_preexec '_command_duration_pre_exec'")
_bash_it_library_finalize_hook+=("safe_append_prompt_command '_command_duration_pre_cmd'")
3 changes: 2 additions & 1 deletion plugins/available/cmd-returned-notify.plugin.bash
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ about-plugin 'Alert (BEL) when process ends after a threshold of seconds'

function precmd_return_notification() {
local command_start="${COMMAND_DURATION_START_SECONDS:=0}"
local current_time="${EPOCHREALTIME:-$SECONDS}"
local current_time
current_time="$(_shell_duration_en)"
local -i command_duration="$((${current_time%.*} - ${command_start%.*}))"
if [[ "${command_duration}" -gt "${NOTIFY_IF_COMMAND_RETURNS_AFTER:-5}" ]]; then
printf '\a'
Expand Down
6 changes: 3 additions & 3 deletions test/plugins/cmd-returned-notify.plugin.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function local_setup_file() {

@test "plugins cmd-returned-notify: notify after elapsed time" {
export NOTIFY_IF_COMMAND_RETURNS_AFTER=0
export COMMAND_DURATION_START_SECONDS="${EPOCHREALTIME:-$SECONDS}"
export COMMAND_DURATION_START_SECONDS="$(_shell_duration_en)"
sleep 1
run precmd_return_notification
assert_success
Expand All @@ -18,7 +18,7 @@ function local_setup_file() {

@test "plugins cmd-returned-notify: do not notify before elapsed time" {
export NOTIFY_IF_COMMAND_RETURNS_AFTER=10
export COMMAND_DURATION_START_SECONDS="${EPOCHREALTIME:-$SECONDS}"
export COMMAND_DURATION_START_SECONDS="$(_shell_duration_en)"
sleep 1
run precmd_return_notification
assert_success
Expand All @@ -34,7 +34,7 @@ function local_setup_file() {
@test "lib command_duration: preexec set COMMAND_DURATION_START_SECONDS" {
export COMMAND_DURATION_START_SECONDS=
assert_equal "${COMMAND_DURATION_START_SECONDS}" ""
NOW="${EPOCHREALTIME:-$SECONDS}"
NOW="$(_shell_duration_en)"
_command_duration_pre_exec
# We need to make sure to account for nanoseconds...
assert_equal "${COMMAND_DURATION_START_SECONDS%.*}" "${NOW%.*}"
Expand Down

0 comments on commit 7c7e4f9

Please sign in to comment.