tavinus / pdfScale

Bash Script to Scale and Resize PDFs using Ghostscript
MIT License
246 stars 37 forks source link

function isFloatBiggerThanZero() is broken #35

Closed jonassmedegaard closed 1 year ago

jonassmedegaard commented 1 year ago

I checked this nice looking script with shellcheck which spewed a range of warnings (that you might take inspiration from as well, but that's besides this issue) and one error:

In pdfScale.sh line 2045:
        isFloat "$1" && [[ (( $1 > 0 )) ]] && return $TRUE
                                 ^-- SC2071 (error): > is for string comparisons. Use -gt instead.

and indeed, if I feed the value "-1" to that function, it returns true.

Here is a replacement for that function which I believe works properly (without turning to bc as it seems you want to try avoid third-party tools):

isFloat "$1" && [[ "$1" =~ ^0*[1-9] || "$1" =~ ^0*[.]0*[1-9] ]] && return $TRUE
tavinus commented 1 year ago

I am not sure we should ever get a negative value in pdfScale's use case.

I would still like to fix the function though, but I would probably add the extra code inside de function.

pdfScale already uses BC for many things. It is already a requirement, so it would not be a problem to use a BC solution (if it is better).

How about just changing > to -gt as suggested?

I will need to test this a bit, before fixing it.

tavinus commented 1 year ago

Ok, was able to test it properly now.

The old function isFloatBiggerThanZero() was indeed busted. Changing to -gt just makes it worse, since it breaks execution when it finds a point (eg 2.8).

Made this little script to test and be sure

#!/bin/bash

TRUE=0                     # Silly stuff
FALSE=1

# Test script for float functions

# Returns $TRUE if $1 is a floating point number (or an integer), $FALSE otherwise
isFloat() {
        [[ -n "$1" && "$1" =~ ^-?[0-9]*([.][0-9]+)?$ ]] && return $TRUE
        return $FALSE
}

# Returns $TRUE if $1 is a floating point number bigger than zero, $FALSE otherwise
isFloatBiggerThanZero() {
        isFloat "$1" && [[ "$1" =~ ^0*[1-9] || "$1" =~ ^0*[.]0*[1-9] ]] && return $TRUE
        return $FALSE
}

# Returns $TRUE if $1 is a floating point number between 0 and 1, $FALSE otherwise
isFloatPercentage() {
        [[ -n "$1" && "$1" =~ ^-?[0]*([.][0-9]+)?$ ]] && return $TRUE
        [[ "$1" == "1" ]] && return $TRUE
        return $FALSE
}

syes() { echo 'yes => '"$1" ; }
sno()  { echo 'no  => '"$1" ; }

v='text' ; isFloat "$v" && syes "$v" || sno "$v"
v=3      ; isFloat "$v" && syes "$v" || sno "$v"
v='1.2'  ; isFloat "$v" && syes "$v" || sno "$v"
v='0.5'  ; isFloat "$v" && syes "$v" || sno "$v"
v='-0.7' ; isFloat "$v" && syes "$v" || sno "$v"
v='-2.5' ; isFloat "$v" && syes "$v" || sno "$v"

echo '----------------'

v='text' ; isFloatBiggerThanZero "$v" && syes "$v" || sno "$v"
v=3      ; isFloatBiggerThanZero "$v" && syes "$v" || sno "$v"
v='1.2'  ; isFloatBiggerThanZero "$v" && syes "$v" || sno "$v"
v='0.5'  ; isFloatBiggerThanZero "$v" && syes "$v" || sno "$v"
v='-0.7' ; isFloatBiggerThanZero "$v" && syes "$v" || sno "$v"
v='-2.5' ; isFloatBiggerThanZero "$v" && syes "$v" || sno "$v"

echo '----------------'

v='text' ; isFloatPercentage "$v" && syes "$v" || sno "$v"
v=3      ; isFloatPercentage "$v" && syes "$v" || sno "$v"
v='1.2'  ; isFloatPercentage "$v" && syes "$v" || sno "$v"
v='0.5'  ; isFloatPercentage "$v" && syes "$v" || sno "$v"
v='-0.7' ; isFloatPercentage "$v" && syes "$v" || sno "$v"
v='-2.5' ; isFloatPercentage "$v" && syes "$v" || sno "$v"

Outputs

$ ./testFloat.sh 
no  => text
yes => 3
yes => 1.2
yes => 0.5
yes => -0.7
yes => -2.5
----------------
no  => text
yes => 3
yes => 1.2
yes => 0.5
no  => -0.7
no  => -2.5
----------------
no  => text
no  => 3
no  => 1.2
yes => 0.5
yes => -0.7
no  => -2.5

Pls note that my isFloatPercentage only returns $TRUE if it is a value from 0 to 1. That is by design for what the script needs. I don't remember if it had to be positive as well 🤣, but if that is the case I can just also run the isFloatBiggerThanZero on top of it.

I will push the changes now. Thanks for the function prototype!

Cheers! Gus

tavinus commented 1 year ago

Fixed on https://github.com/tavinus/pdfScale/commit/1edc9d548f697d1ec77237764ea8ed2b33c7719c v2.5.8