Openrefine: Clipboard imports now get named as "(clipboard)" instead of "clipboard"

[wetneb]

Describe the bug

One side effect of #2720 is that clipboard imports now get named "(clipboard)" by default instead of "clipboard". Was this intended?

Since this is not something I was expecting I would propose to only include this in 3.5, not 3.4, since there is a risk that there are other unforeseen consequences. Not that I think this particular change is necessarily bad (we might actually want to keep the brackets), just that this is not something I was expecting.

To Reproduce
Steps to reproduce the behavior:

1. Type in anything in the clipboard import area
2. Validate
3. See the default project name in the top right text input

Current Results

The project name is "(clipboard)" by default.

Expected behavior

So far "clipboard" was the default.

Additional context

This new name seems to be generated in the backend, so I was about to propose to localize it but unless we manage to generate it in the frontend instead, it might not be doable at the moment.

[tfmorris]

That's weird. It was not intended and I can't think of what would cause it. Are you sure it's related to #2720?

[wetneb]

I tested before and after the commit and observed a difference… I have not investigated further to find the root cause though.

[tfmorris]

I see what's going on. It was always "(clipboard)":

https://github.com/OpenRefine/OpenRefine/blob/749704518c330c6f96ec3e784c45b8c055d5c6cd/main/src/com/google/refine/importing/ImportingUtilities.java#L249

but the more stringent filter was getting rid of the parentheses. Now that we've relaxed the filter, they make it through.

[tfmorris]

I think changing this on the backend introduces more risk than it removes. In particular, it will require adjusting tests.

I'm not 100% convinced that it needs fixing at all, but in the interests of getting the release out the door, here's the line that needs adjusting:
https://github.com/tfmorris/OpenRefine/blob/55171a85ebf0b5145d295b5f18e6805070e0b989/main/webapp/modules/core/scripts/index/default-importing-controller/parsing-panel.js#L63
[not sure why the permalink isn't rendering code]
We could add parentheses "(" ")" to the list of characters which aren't legal in project names or special case the string "(clipboard)".

Preferences?

[wetneb]

I am fine with either tweaking the frontend code or the backend - I also think it would make sense to restore the original behaviour for the sake of uniformity.

[thadguidry]

Since this seems to only affect new projects made? And old projects would still load just fine with a name like "clipboard" (like the 100's I have), then I think leaving it as new updated "(clipboard)" name is fine for users. It definitely is fine for me if it pops up when I next create a project from a clipboard. As a user, I personally don't see the parentheses ( ) as a problem.

[ostephens]

We could add parentheses "(" ")" to the list of characters which aren't legal in project names or special case the string "(clipboard)".
I'm definitely against this

Adding the parentheses seems slightly odd, but I don't think it matters a huge amount. I'm happy to see them left in for now

[wetneb]

"For now"? Surely if we want to revert that back at some point, it should be done now, to avoid releasing things that we roll back later, no?

[ostephens]

I really don't like the solution to trim the parentheses - that seems like we prevent the user doing something they may want to do (have parentheses in a project name) to work around a quirk of programming

I think we should either not include the parentheses when we name the project, or not worry about it

I'm happy to not worry about it (for now or forever)

[wetneb]

Ok, I agree. For me the safest way forward would be to remove the commit which created this change from 3.4, leave it in master, and so 3.5 will have "(clipboard)" instead of "clipboard" and that will be fine. That does not require any messing around with either the frontend or the backend, and follows the principle of only including safe fixes with no unintended consequences in a release branch.

[tfmorris]

It's hard to believe that a purely cosmetic minor change to a field default value generates such a volume of discussion. The user can change the default to anything that they want.

Reverting the change means that only ASCII alphanumerics will be included from filenames. No multinational characters. No punctuation, such as the " (2)" that Mac OS X adds to disambiguate duplicate filenames. It will undo the fix to the 3 year old bug report about multinational character project names #1352.

The PR #2812 is misleadingly named because it strips parentheses from all filenames (including the above mentioned ones generated by OS X), not just "(clipboard)".

Yes, risk management for minor releases is important, but I'm having a hard time viewing this as a significant risk. If we're going to change it, though, the change should be an improvement. Since the value needs to be translatable anyway, I've special cased it to replace the internal "(clipboard)" with the translated value for "Clipboard," so French users will see their default clipboard project name as "Presse-papier" rather than "clipboard." #2776

[tfmorris]

add parentheses "(" ")" to the list of characters which aren't legal in project names

This was poorly worded. It's actually the list of characters which are stripped from filenames when creating the default project name. I'm not actually a fan of stripping any characters, but rather than remove it altogether, the list has been trimmed to a minimal set.

In any case, we still need to internationalize hardcoded English values.

[wetneb]

Sorry, but it is my role to be a bit stubborn on this:

• this patch introduced an unexpected change that has high visibility to the user
• it generates a high volume of discussion indeed
• it does not address a bug that was introduced recently

so it should not be introduced between a beta release and a final one.

It is not the end of the world if this only gets released in 3.5 - we have lived with the previous behaviour for years.

Also, now that I think of it, it is not clear to me why we should strip any characters from project names at all.

[wetneb]

Sorry if it feels a bit feels a bit rigid, but I have sore memories of previous failures to argue for the stability of releases, most importantly the change in the list facet that we released in 3.1 which was an epic fail. I understand you want your patches released as soon as possible, and as you can see in other PRs I am not systematically that stubborn!