[bug] Or and And words don't make their jobs

[Olyno]

Description

Recently I saw when working on #22 that the and and or operators didn't work correctly sometimes.

Expected

Should be false: true
Should be true: true
Should be false: false
Should be false: false
Should be true: true
Should be true: true
Should be true: false
Should be false: false

Code:

on script load:
    set {_x} to true and false
    print "Should be false: %{_x}%"
    set {_x} to true and true
    print "Should be true: %{_x}%"
    set {_x} to false and false
    print "Should be false: %{_x}%"
    set {_x} to false and true
    print "Should be false: %{_x}%"
    set {_x} to true or false
    print "Should be true: %{_x}%"
    set {_x} to true or true
    print "Should be true: %{_x}%"
    set {_x} to false or true
    print "Should be true: %{_x}%"
    set {_x} to false or false
    print "Should be false: %{_x}%"

It's good to note using operators as && instead of and it works perfectly. No errors, simply wrong results.

[APickledWalrus]

Trying to set a single variable to multiple values shouldnt error, not just select one of them, no?

[Olyno]

It should select one of them in fact (see the getValues method of the expression)

[Syst3ms]

This issue is due to the %boolean% or %boolean% syntax conflicting with literal lists of the form object 1 or object 2. I have some ideas about how to fix this, such as adjusting the syntax for the literal list case, but it might prove to be unwieldy and unnecessarily complicated.
I'm also reluctant to adding beginner-unfriendly shorthands, as I mentioned in #20. However if absolutely necessary, I might resort to that option.

[Mwexim]

Why not give priority to single value expressions if one is asked?

For example:

{a} = true and false # Results in false because it prioritises the condition operator expression because it is single
{b::*} = true and false # Results in the list 'true and false' because it's not single

Then if the user wants to set a list to a single value using the condition operators, it can just use the workaround of setting it to a single temporary variable first and then using that variable to set the list variable.

Does that make sense?

[Syst3ms]

That could work fine for variables, but for other changeable expressions less so. Imagine a singular expression that allows being changed using multiple values : which one should be picked in such a case?
Regardless, this solution isn't the most elegant in my eyes.

[Mwexim]

That could work fine for variables, but for other changeable expressions less so. Imagine a singular expression that allows being changed using multiple values : which one should be picked in such a case?
Regardless, this solution isn't the most elegant in my eyes.

Just looked at the change(...) code. What if we make the method signature change(Duration[] toChange, Expression<Object> changeWith, ChangeMode mode) (if that is even possible)?
If we do that, it should be possible for the author to check if the Expression is single or plural, shouldn't it?

Either way, I just looked at some of the code and found some redundancy that may have to do with this issue:

    @Override
    public Class<?>[] acceptsChange(ChangeMode mode) {
        if (!list && mode == ChangeMode.SET)
            return new Class[]{Object[].class};
        return new Class[]{Object[].class};
    }

(Variable.java)
This is pretty redundant if you ask me :)

[Syst3ms]

If we do that, it should be possible for the author to check if the Expression is single or plural, shouldn't it?

Well, no, since that singular/plural distinction has to be made much earlier at parse time (in fact it has to be made at matching time), and acceptsChange isn't called until much later in the process.

[Olyno]

I guess what I found is related to this issue:

When creating the pull request #60, the getValues method gave me only the first string (expression) it find. For example:

Expected:

set {_upper::*} to all upper case chars in "Hello World" and "Hello Parser"
print "Upper: %{_upper::*}% # Upper: "H", "W", "H" and "P"

Got:

set {_upper::*} to all upper case chars in "Hello World" and "Hello Parser"
print "Upper: %{_upper::*}% # Upper: "H", "W" and "Hello Parser"

[Syst3ms]

That's because list literals come before expressions in the parsing order. Use parentheses to specify the correct interpretation.

[Mwexim]

That's because list literals come before expressions in the parsing order. Use parentheses to specify the correct interpretation.

Maybe force-parsing (as I will call it for now) for ExprBooleanOperators could fix the issue?
What I mean by force-parsing is something that was present earlier for inline conditions: there is a separate section that parses boolean operators first, and then the other expressions follow (in the SyntaxParser class).