search

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

Replace regex with procedure #571

Closed jeremypw closed 10 months ago

jeremypw commented 1 year ago

Fixes #566

Instead of using a regex to match the whole of the input, each component expression is checked for validity using a procedure.

For now, for simplicity, use of variables is disallowed. It would be possible to allow variables at the cost of more code complexity if it was thought worth it in this context.

Apart from variables the capabilities should be the same as in master.

All the expressions passed by the procedure should give a valid result when passed to bc.

An additional check is made compared to master (or bc) which is that if an ibase is set then invalid digits for that base are excluded.

peteruithoven commented 11 months ago

@jeremypw thanks for working on this. Why keep this as a draft? Is it because variables aren't allowed? That seems like an okay compromise to fix the error.

jeremypw commented 11 months ago

@peteruithoven I had forgotten about this! It looks like I was waiting for feedback on whether it would be necessary (or desirable) to allow variables but as I don't think they are allowed in master, I'll open it for review.

Correction: Variables are usable to some extent in master but I am not sure they are very useful in the context of the applications menu so I'll leave open for review.

jeremypw commented 11 months ago

Converting to draft while adding additional unit tests.

jeremypw commented 11 months ago

There is a problem with using "obase=" - the output is expected to be a double so a result such as "FFFF" cannot be output and binary numbers are misinterpreted. I'll see if there is a way round this.

peteruithoven commented 11 months ago

Cool, seeing all these assets_throw being changed to assert_equal means you've enabled much more input right?

jeremypw commented 10 months ago

Cool, seeing all these assets_throw being changed to assert_equal means you've enabled much more input right?

Yes, that's right.

jeremypw commented 10 months ago

@peteruithoven I would prefer to deal with possible stripping of trailing zeroes in a separate PR if it is thought to be an issue. It is not completely trivial to make it work in all locales and I do not want to cause regressions in this PR.

peteruithoven commented 10 months ago

Thanks for all the work on this @jeremypw !

jeremypw commented 10 months ago

Thanks! I was thinking of adding a placeholder or something to make this and other functionalities more discoverable.

UPDATE: There is a placeholder but it doesn't normally show because the search entry has focus when opened. Maybe a tooltip?