Don't regexp-quote twice

[tdrhq]

This was first introduced in c53e699aa4340d93f1784b79e7280390247af721 cc @svetlyak40wt

I can't tell if there's a specific purpose to the multiple regexp-quotes, but currently it fails on things with '.' (for instance, if I import multiple things from bknr.datastore it'll create separate :import-from for this. e.g. https://github.com/screenshotbot/screenshotbot-oss/blob/177e189ad3b2dc3811c17435013076e893959512/src/util/store-version.lisp#L9

There's one more bug in this regexp that I want to fix in a follow commit.

[tdrhq]

Thoughts on this pull request? I want to contribute and send more fixes, but it's discouraging to see a simple fix not being merged.

[joaotavora]

Sorry, i have limited time available and things slip through. Things watch rewrite more thinking and or dont immediately indicate errors to the user are of less priority. Keep pinging.

[tdrhq]

Are you looking for additional reviewer support on PRs? Happy to help out a bit. I wouldn't say I'm an expert in Sly, but I've dabbled enough in its internals, and very dependent on it. Obviously, if it's a philosophical thing about "not changing what works", then never mind. (I know a lot of CL projects follow that principle, for better or worse.)

[joaotavora]

If you want to speed up and increase the chances of merging this PR (or any PR) I need one or more of the following

1. A reproduction recipe, a Minimum Reproducible Example, from Emacs -Q as described in the CONTRIBUTING.md file demonstrating what happens that you don't like and what you think it should behave.
2. Commit messages that follow the GNU Changelog Format style. Just see other commit messages.
3. No spurious whitespace changes like you have in your commit.

The clearer these things are, especially number 1, the easiest it is for me to merge a patch. IOW, it's not enough to notice that there are two chained regexp-quote. Is it a performance hog? Is it breaking anything? I see there's some text about something unexpected in your description, but I can't easily make out what it is. I prefer not needing to look at screenshots or learning what bknr.datastore is, for instance.

[tdrhq]

@joaotavora That makes sense, I understand that keeping code easy for review is an important responsibility on my part. Let me make those changes, and I'll get back to you in a few minutes.

[tdrhq]

Oh, I didn't mean to delete this branch, I was renaming branches locally. I'll push a few changes in a second

[tdrhq]

Updated this PR, with a proper test demonstrating the issue.

Please note that this PR depends on PR #591. GitHub PRs don't handle dependencies very well, so it might be beneficial to merge #591 first. Or just look at the last commit on this PR.

[joaotavora]

GitHub PRs don't handle dependencies very well,

Just put two (or more) commits in one of these PRs (and close the other(s)).

[tdrhq]

I want to make sure you can review both independently. They're two independent changes. Just making your life easier as a reviewer. The first change is a noop, so I don't want it to be blocked on changes in this PR which actually changes behavior.

[joaotavora]

I appreciate the effort, I really do, but please do both commits in one PR. It is easier for me, in this project, it really is. I would know, i've worked here for a while, SLY is turning 10 one of these days :-) And in the PR where you add tests, add also the test that fails here. Thanks in advance, Arnold

[tdrhq]

Okay, I've closed the other PR, so this PR should have all the content that is to be reviewed. Again, the actual fix is in the last commit on this PR, the previous commits just set up the test infrastructure

[joaotavora]

I pushed your fix and your tests (with some adjustments). Have a look . And thanks!

[tdrhq]

Much appreciated <3

I'll send one more small PR fixing one more frequent bug with this logic

[joaotavora]

OK. I found two more bugs myself when testing this.