No substitution for (or with) certain strings

[kandread]

Hi @protesilaos! Thank you for (another) great package. I was testing it out and encountered two issues with the example

var balmoral = /* color: #ffffff */ee.Geometry.Point([172.622101, -42.7989879990594]),
    ashley = /* color: #ffffff */ee.Geometry.Point([172.560061337, -43.2241230890585]);

1. with the cursor on color and attempting to replace with an empty string doesn't seem to work
2. attempting to replace the comment part /* color: #ffffff */ doesn't do anything either.

I was able to trace the first issue in substitute--prompt-without-highlight with the default value in the read-string call (just used pretty-target replacing target with itself). The second issue seems to be related to the escape sequences, which cause re-search-forward to not find any matches of target. I've fixed both issues in my fork, but not sure what I did was the best option. Just wanted to bring these to your attention, and happy to create a PR.

[protesilaos]

Hello @kandread!

Thank you for (another) great package.

You are welcome!

1. with the cursor on color and attempting to replace with an empty string doesn't seem to work. [...] I was able to trace the first issue in substitute--prompt-without-highlight with the default value in the read-string call (just used pretty-target replacing target with itself).

I am aware of this and your findings are correct. This is the minimal scenario (writing it here in case another person reads this):

(read-string "Test with empty value: ")
(read-string "Test with empty value: " nil nil "Hello")

This is a tricky case. My instinct is to treat the empty input as "I am giving you an empty string" but in Emacs this action usually means "Return the default value". I think for substitute it makes more sense to follow our intuitions of replacing something with an empty string. But I have not committed to that because there may be a canonical way of doing it and I have not discovered it yet.

2. attempting to replace the comment part /* color: #ffffff */ doesn't do anything either. [...] The second issue seems to be related to the escape sequences, which cause re-search-forward to not find any matches of target. I've fixed both issues in my fork, but not sure what I did was the best option. Just wanted to bring these to your attention, and happy to create a PR.

In principle, I agree wih fixing both and I am happy to merge your PR. Though given what I wrote above, I say we first merge the fix for 2 and then give ourselves a chance to rethink what we should do with 1. Just so that we conform with Emacs conventions.

What do you think?

[kandread]

Agreed, and it's exactly the reason I opened up an issue to get your feedback instead of submitting the PR right away. I will close the issue, but feel free to keep it open of course.

[protesilaos]

Very well! Please prepare the PR for case 2. I think we should do it for 1 as well, but let's think about it a little more.