dshoreman / nextshot

A simple tool for taking screenshots on Linux and sharing via Nextcloud
GNU General Public License v2.0
42 stars 3 forks source link

Nextshot doesn't work correctly if locale does not use '.' as decimal separator #64

Closed e-alfred closed 4 years ago

e-alfred commented 4 years ago

Hello,

I can't take a screenshot of an area or window if the numeric location isn't set to en_US.UTF-8 because BC doesn't seem to be able to do the calculation in a correct way:


$ nextshot -t
/usr/bin/nextshot: Line 343: printf: 1.00000000000000000000: Invalid Number.
/usr/bin/nextshot: Line 343: printf: .39215686274509803921: Invalid Number.
/usr/bin/nextshot: Line 343: printf: .70588235294117647058: Invalid Number.
Starting Nextshot tray menu...

If I start Nextshot using LC_NUMERIC="en_US.UTF-8" /usr/bin/nextshot -t everything works perfectly fine.

dshoreman commented 4 years ago

@e-alfred If you open a fresh terminal without setting LC_NUMERIC, what's the output of locale?

e-alfred commented 4 years ago

Currently it is this:

LC_NUMERIC="de_AT.UTF-8"

dshoreman commented 4 years ago

Thanks, when I'm back in linux I'll do some testing with your locale and see if I can replicate. My guess is the European decimal separator being a comma instead of a dot is confusing things somewhere.

Hopefully it'll be as simple as converting to/from the local decimal format... Watch this space!

On Thu, 2 Apr 2020, 12:36 e-alfred, notifications@github.com wrote:

Currently it is this:

LC_NUMERIC="de_AT.UTF-8"

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dshoreman/nextshot/issues/64#issuecomment-607791583, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALTRCWJ4GLLGKLFJULARK3RKR2EHANCNFSM4LXBY4DA .

dshoreman commented 4 years ago

@e-alfred I've been able to replicate this in a VM and solve it, thankfully without requiring en_US.

I'll release a new version with the fix shortly, thanks again for reporting the issue!

dshoreman commented 4 years ago

Fix has been pushed to master, and should show up in AUR as v1.3.2 shortly if you're on Arch.

If you still have the problem with the update, please reopen this issue