pop-os / launcher

Modular IPC-based desktop launcher service
Mozilla Public License 2.0
228 stars 45 forks source link

Remove "approx." keyword and improve decimal point/comma handling #70

Closed nathansgithub closed 2 years ago

nathansgithub commented 2 years ago

This changes the calculator behavior to stop showing "approx." in front of numbers when it has more decimal places than can be shown, by enabling qalc's Unicode support. It also explicitly sets the decimal mark to decimal points when that's preferred over commas.

For issue #71

13r0ck commented 2 years ago

Thank you for the PR! It seems though that this bug doesn't exist in Pop!_OS 21.10. As that version will be released in < 24 hours, it doesn't make sense to merge this PR.

jacobgkau commented 2 years ago

It seems though that this bug doesn't exist in Pop!_OS 21.10. As that version will be released in < 24 hours, it doesn't make sense to merge this PR.

We also want to consider Pop!_OS 20.04 LTS as well as other distributions. It looks like this is still happening in 20.04 LTS, so we do want to review the PR.

jacobgkau commented 2 years ago

Oh, I see, you were saying the ≈ is stripped the same was a = currently is. I think this would be a good change, if we can get the ≈ to show up in the result visually.

nathansgithub commented 2 years ago

@jacobgkau Yeah, I meant it uses the ≈ symbol behind the scenes. Do you think it's important to distinguish an approximate result, and how would you want to do that? I do want to keep the use case where you can hit return and immediately use the previous result as part of a new calculation (and that's the main reason I suggested this change).

jacobgkau commented 2 years ago

I do want to keep the use case where you can hit return and immediately use the previous result as part of a new calculation

100% agreed on that point, that is an intended feature.

Do you think it's important to distinguish an approximate result, and how would you want to do that?

It does seem important. I would think just having at the front of the result would be sufficient. (I'm pretty sure that was the behavior at some point; it might have changed when it moved from MathJS in pop-shell to qalc in pop-launcher.)

If making the launcher treat as a prefix for this plugin is the simplest way to accomplish that, then that sounds fine (pending engineering review, of course.) The other option would be getting it to only bring the rest of the result after the into the search field when pressing enter.

nathansgithub commented 2 years ago

@jacobgkau I think this fixes it. It will leave the ≈ in front of approximate results but keep the same behavior for other results. The existing code already filters out ≈ before the value is sent to the Fill() function that fills the search field.

nathansgithub commented 2 years ago

I upgraded my system to 21.10 and noticed some changes in qalc's formatting, so I added a couple changes to the code.

  1. It is showing the calculation in multiple steps when the fractions can be simplified. So I changed the code to look from the right of the string for ≈ or = instead of from the left. Examples of equations that this helps normalize:
    7 / 6 = 1 + 1/6 ≈ 1.166666667
    100 / 40 = 5/2 = 2 + 1/2 = 2.5
  2. The maximum digits after the decimal increased from 7 to 9. I explicitly set that to 9 so tests are consistent.
nathansgithub commented 2 years ago

@jacobgkau Let me know if there's anything else you want me to change. I think this PR will make the results clearer on both 21.10 and 20.04. Right now, the equation 7/6 resolves to: approx. 1.1666667 in 20.04 (the last I checked, unless qalc was updated there) 1 + 1/6 ≈ 1.166666667 in 21.10

This change will make it so the answer on both is just ≈ 1.166666667, which I think makes more sense and gives a more consistent behavior if you are making several calculations based off each other in the launcher.

Thanks!

nathansgithub commented 2 years ago

Awesome, thank you!