Stefan Monnier writes: >> Two things: >> >> - It should have somewhat better performance because we're running >> try-completion on smaller strings and therefore doing less string >> comparison and work. > > Do we have concrete evidence to believe this? No evidence. So nevermind the performance argument, I suppose. >> CCS previously contained, for each completion, a list containing one >> element for each wildcard element of PATTERN. >> >> Now CCS contains, for each completion, a list containing one element for >> each element of PATTERN, both fixed strings and wildcards. > > Exactly: that means CCS is more costly to build and AFAICT it also > implies correspondingly more calls to `try-completion`. > > I don't find it obvious that the new code will be more efficient. > Barring any clear evidence that one of the two versions is noticeably > more efficient than the other, I'd recommend we stick to the code we > happen to have. > > From where I stand, my impression is that neither version is terribly > better nor terribly worse than the other, in terms of clarity, > simplicity, and efficiency. That's fair, that patch alone is not worth much. Attached is a patch (which applies on master) which does most of the rest of the refactoring that I wanted to do; it makes the dolist over the pattern into a simple mapcan over the pattern, with no state preserved between elements of the pattern. IMO, this is much easier to follow (and I think it would make the various bugs I've fixed in the last couple years in merge-completions, easier to catch). What do you think? > PS: BTW, there's currently a limit of 255 subgroups in regexps, which > for `flex` currently implies a limit of 254 chars and with your patch > that limit would be halved. Oh, I didn't realize that. I could definitely lift that limit on flex with my design, though, so that it's not limited to 254 chars. We can just do repeated string-searches instead for each of the fixed elements, which won't have this limitation.