BashScripts icon indicating copy to clipboard operation
BashScripts copied to clipboard

bash-timer: ',' as decimal separator + "invalid integer" error

Open loreb opened this issue 1 year ago • 1 comments

So, a couple of bugs in just one issue as one of them is teeny tiny...

Create an empty HOME with eg cd $(mktemp -d) && cp /etc/skel/.bashrc . && exec env - TERM="$TERM" PATH="$PATH" HOME="$PWD" bash

Install as per README (curl...|bash), run bash...

If the locale uses ',' as decimal separator (eg export LC_ALL=it_IT.UTF-8) the code doesn't work because there's no '.' in $EPOCHREALTIME (I've added a silly case..esac locally)

More importantly, and without messing with the locale, try sleep 0.$((RANDOM&1023)), and occasionally you'll get bash: 10#: invalid integer constant (error token is "10#")

PS Thank you! This functionality was on my TODO list :D

loreb avatar Aug 26 '24 18:08 loreb

Why can't the world standardize on "." as the decimal separator? It's almost as inane as the 100 different electrical outlet sockets when we could standardize on universal sockets quite cheaply for all new construction.

hopeseekr avatar Aug 29 '24 12:08 hopeseekr

Because '.' is used as the thousands separator.

Seriously, it was weird to me as well as a computer person - I installed linux on $JOB laptop, kept the locale as yet another visual clue whether I'm local or in a ssh session, and boom! Yet more seriously, the fix is incorrect - if I set -x; bashtimer_preexec ;true;bashtimer_precmd I see eg

+ EPOCHREALTIME=1724959197.540153
+ end_s=1724959197,540201
+ end_ns=1724959197,540231
+ end_ns=1724959197,540231
+ begin_ns=539828
+ end_ns=540231
+ s=415

I'm afraid you'll have to add a temporary variable.

loreb avatar Aug 29 '24 19:08 loreb

I fixed the comma decimal problem easily enough. You can download the latest bash-timer at hopeseekr/bash-timer. The latest trunk has the fix.

The other math problem is very hard to fix. I've arleady spent hours and hours on it. No doubt, the original math error took me over 4 years to hunt down and fix.

ChatGPT already failed to find the issue, so I'm going to consult Claude 3 Opus now...

hopeseekr avatar Sep 01 '24 16:09 hopeseekr

I've been testing it on es_CO.UTF-8 and it seems to work well enough??

UPDATE: Never mind... there must have been a reversion before I published. It currently isn't working.

hopeseekr avatar Sep 01 '24 16:09 hopeseekr

  • [2024-08-29 07:49:02 CDT] [bash-timer] Added support for locales with the comma decimal separator.
  • [2024-09-01 12:31:33 CDT] [bash-timer] Properly pad ms to 3 digits.
  • [2024-09-01 12:41:07 CDT] [bash-timer] Greatly refactored ns math to properly calculate seconds and ms.

I've thoroughly tested it in both ar_AE.UTF-8 and es_CO.UTF-8 and it's working well with both the Arabic and comma-based decimal separator now. I believe those are the only three major decimal points in the world.

This app explicitly supports Hindi, Chinese and Spanish. Hindi and Chinese use the period ("."), and Spanish countries almost exclusively use the comma (",").


I also fixed the ms logic when the end ms is less than the begin one.

hopeseekr avatar Sep 01 '24 17:09 hopeseekr

Unless I made some silly mistake, the ms issue is 99% fixed, while the locale isn't... bash(1) says "Assignments to EPOCHREALTIME are ignored", which means that you must use a temporary variable, as in local E=${EPOCHREALTIME/,/.}; end_ns=${E#*.} etc.

Regarding the ms issue, github.tar.gz see the attached file - it's bash-timer.sh, slightly modified to be executed trying all possible values; unless my machine is cursed, it fails for me when it tries to add some number and 08 (or 09), which it interprets as octal (I didn't know until a few minutes ago!).

I mean, I know it's not a patch, but at least it should be reproducible... if it isn't I swear I'll grok the thing and make a patch myself this weekend.

loreb avatar Sep 02 '24 18:09 loreb

Brute force solution, probably wrong but I'm too tired to debug why (time sleep 0.$((SRANDOM % (1<<20) )) => time says it takes longer than in the prompt, but a silly C program to measure the time disagrees with that, and I'm too tired to figure out why).

Anyway the big idea was just to trim two '0', which does nothing if there aren't two '0' in the first place - see attachment (sorry). 0github.tar.gz

loreb avatar Sep 02 '24 21:09 loreb