elementary / applications-menu

Applications Menu for elementary OS and the Pantheon desktop environment
https://elementary.io
GNU General Public License v3.0
104 stars 35 forks source link

Searching "next" shows calculated 0 as result #566

Closed peteruithoven closed 10 months ago

peteruithoven commented 1 year ago

What Happened?

When searching for "next" (or "Firefox beta") it shows calculator output 0 as first result. Screenshot from 2023-06-10 01 09 05@2x

Steps to Reproduce

  1. Open applications menu
  2. Type "next"

Expected Behavior

No calculated result is shown

OS Version

7.x (Horus)

Software Version

Latest release (I have run all updates)

Log Output

No response

Hardware Info

No response

peteruithoven commented 1 year ago

Looks like we replace the "x" with "*": https://github.com/elementary/applications-menu/blob/master/src/synapse-plugins/calculator-plugin/calculator-plugin-backend.vala#L60

Afterwards "ne*t" matches the following regexp: https://github.com/elementary/applications-menu/blob/master/src/synapse-plugins/calculator-plugin/calculator-plugin-backend.vala#L44 Example: https://regex101.com/r/URh3rG

peteruithoven commented 1 year ago

For reference, current reg exp: ^.*(\w+[\/\+\-\*\^\%\!\&\|]{1,2}\.?\w+|\(-?\d+.*\))+.*$. What if we go with something like: ^([-\d.]+[\/\+\-\*\^\%\!\&\|]{1,2}[-\d.]+)+$

Example: https://regex101.com/r/aSz5MI

Would be really great to have some unit tests on this part, because I'm probably forgetting about many usecases. Update: I should try this with the existing unit tests. Update 2: My reg exp fails 50% of the calculator tests :sweat_smile:

peteruithoven commented 1 year ago

Looking for "Firefox beta" also returns the 0.

jeremypw commented 1 year ago

Looks like the "x" is interpreted as "multiply" even though the other parts are non-numeric and presumably parse to "0". Should be easy enough to check the non-operator parts are actually numbers?

jeremypw commented 1 year ago

Firefo^ Beta = 1 (interpreted as 0 ^ 0).

jeremypw commented 1 year ago

As bc can handle multiple expressions it is possible for next to be a valid expression: e.g. ne=3;t=4;next gives the expected result 12. The code in this app is too simplistic to handle this kind of thing. It just passes a string that might be a valid mathematical expression(s) to bc and returns the result if bc gives no error. Unfortunately bc does not give an error for undefined variables - just assigns them the value 0. Whether it is worth us trying to check for undefined variables here is questionable. It would be simpler to disallow variables altogether.