firasdib / Regex101

This repository is currently only used for issue tracking for www.regex101.com
3.24k stars 199 forks source link

Substitution String Not Saved #1889

Closed CarlitoGil closed 1 year ago

CarlitoGil commented 2 years ago

Bug Description

"Update Regex" does not save the substitution string when the currently selected function is not "Substitution".

Reproduction steps

  1. Create a new regex with pattern "^([A-Z]+)([0-9]+)$".
  2. Select "Substitution" Function.
  3. Write substitution string "$1".
  4. Save the regex.
  5. Select "Match" function.
  6. Write test string "ABC123".
  7. Update the regex.
  8. Select "Substitution" Function: "$1" is still there.
  9. Refresh the page (F5).
  10. Select "Substitution" Function. The substitution string is empty.

Expected Outcome

The substitution string should be saved even if the "Substitution" Function is not currently selected.

Browser

Chrome 105.0.5195.102 x64

OS

Windows 10 x64

working-name commented 2 years ago

Hello! I can reproduce the behavior given the steps, however, I'm not sure if this is a bug or simply architectural. Firasdib will have useful input.

firasdib commented 2 years ago

The editor saves the "current" state only. If you toggle off substitution, then it won't be saved. This is because its ambiguous what to save if you have used all the different functions.

Can you suggest a better way of handling this?

CarlitoGil commented 2 years ago

The unit tests are saved even if another function is selected. Similarly, the Test String is always saved even if the Unit Test function is selected. I would expect the same of the Substitution and List strings.

If the regex is meant to have a Substitution string, but the user is working temporarily with the Match function selected (to view more, for example), the substitution string is lost when saved. It's inconsistent that you can go back to the Substitution function and the string is still there before refreshing the page, but not after.

The current saving behavior is as unintuitive as deleting the substitution string just by switching to another function. If the editor is meant to save the current state, then it's failing. When saving/updating with the Match function selected, the current state still has the substitution string when you switch back (as it should) but it wasn't saved.

My suggestion is simply that the content of all non-empty fields should be saved, even if they are currently out of view; and the function selected when saving should be active after refreshing the page.

firasdib commented 2 years ago

I understand. I will experiment with it and see how it works out!

firasdib commented 2 years ago

One clumsy aspect to always saving is there is no "off" switch, i.e., when to determine if the user wants to remove the substitution data altogether and save.

I think this can be resolved with the following logic:

  1. If there is text in the field(s), save it
  2. If there is an empty string, only save it if its currently active

How does that sound?

CarlitoGil commented 2 years ago

Seems OK by me, but honestly, I don't understand why it needs #2.

If the field is now empty, must be because the user deleted the string, no?

If it's empty it should be saved empty, regardless of which function is now selected. That's what I would expect if I wasn't familiar with the app.

firasdib commented 2 years ago

Let me elaborate.

When you save a substitution string, that function is automatically selected when you load that link the next time. If I don't have the second case, I can never determine when I should or shouldn't show that function.

CarlitoGil commented 1 year ago

I can never determine when I should or shouldn't show that function.

Why not use a separate variable for which function was active at the time of saving? Wouldn't be more than a byte.

firasdib commented 1 year ago

The intention was to try to show the user the most relevant information when they load the page. I.e., if there is no test string, just unit tests, I would show that, etc.

Its not a space concern.

CarlitoGil commented 1 year ago

I'm just one user, but for me at least, the most important information is to resume wherever I was when I saved.

Perhaps the alternate logic you describe is best for when someone that's not the owner opens a link that was shared with them. In that case, if the function where it was saved is empty, the app opens on the non-empty function with the highest priority.

Example: if the owner saved with the Substitution function active, but empty, for a guest it would default to the Match function, but if Test String is empty and Unit Tests exist, go there.

firasdib commented 1 year ago

A lot of expressions are shared, so the behavior has to cater to both use cases.

Empty substitution strings are very common.

CarlitoGil commented 1 year ago

I think it would be OK for the users either way, as long as non-empty strings aren't lost when saving.

firasdib commented 1 year ago

Great! Then I will try it out with the method I posted earlier, and see how that works. If it causes any errors, we can look at alternatives.

Thank you.