Just FYI, I am happy with the spirit of the PR, but just note that we're about to transition over to the next version. And since this has such a large scope-of-effect since this touches fundamental selection logic, this will be deferred until after we release (or branch for) 3.2.

Seems like 95% of this PR is code changes that are just refactoring and not needed to support this feature? If so, can this be split into multiple commits or PRs?

I rather prefer to increase the test coverage in this area than revisit code that is already done. May this be an option?

@JarrettSJohnson may this code be accepted with additional test cases? Or should I really split this commit into refactoring and fixing #470?

I didn't know this blocked the other PR. But I think either way, most of this PR still touches too much selection code--even if you were to add additional tests, who knows what else could regress that we haven't considered. IMO, this will probably have to wait until the next cycle.

Alright, let keeps this in standby mode...

3.2 is getting pushed back a bit more, so I'm good with getting these changes in. I think this PR is mostly fine, but again the fix and refactor should be split into separate commits. If there's some difficulties in doing so, I don't mind helping you out with that.

Hi @JarrettSJohnson. How do I write and test the following case?

#include <string>
#include <vector>
#include <catch2/catch.hpp>
#include "layer3/Selector.h"

TEST_CASE("Selector", "[PARSER]")
{
    std::vector<std::string> tokens;
    
    tokens = SelectorParse(NULL, "chain A and resi 10-20 or resn ATP");
    REQUIRE(tokens.size() == 8);
    REQUIRE(tokens[0] == "chain");
    REQUIRE(tokens[4] == "10-20");

    tokens = SelectorParse(NULL, "x >= 5.0 and y < 10.0");
    REQUIRE(tokens.size() == 7);
    REQUIRE(tokens[1] == ">=");
}

Support for C PyMOLGlobals-dependent tests are still not available (passing NULL to SelectorParse will try to dereference it when Feedback is called--so just avoid doing this). So feel free to just stick with the Python tests for now. The PR can contain both commits if you want, btw. I just wont squash them when I merge.

I was since the morning fighting with the code including a syntax error (unbalanced parenthesis) and other "IDE-time" errors my environment didn't catch. I'm lucky to have Gemini that solved my problems right in.