Added wildcard support to groovyw <item> get

[BenjaminAmos]

Description

This PR adds wildcard support to the groovyw <item> get <object> e.g. groovyw <item> get *.

Testing

Testing this PR could take some considerable time and effort to check all of the use-cases are correct, so these changes should only apply if wildcards (* or ?) are detected in the input argument (see Line 61 of util.groovy). As such, I will list the series of tests that I have tried so far:

Single argument tests:

• Command: groovyw module get *
  • Expected result: All the available modules under the @DestinationSol organisation should be obtained.
• Command: groovyw module get d??p*
  • Expected result: This should obtain the deepspace module.
• Command: groovyw module get s??????
  • Expected result: This should obtain the salvage module.
• Command: groovyw module get f?rm?c
  • Expected result: This should obtain the formic module.
• Command: groovyw module get t*
  • Expected result: This should obtain the tribe module.
• Command: groovyw module get *de*
  • Expected result: This should obtain both the deepspace and the federal modules.

Multiple argument tests:

• Command: groovyw module get *de* fo*
  • Expected result: This should obtain both the deepspace, federal modules, as well as the deepspace and the formic module.
• Command: groovyw module get f* d???*
  • Expected result: This should obtain the deepspace, draconic, federal and formic modules.

Testing regex characters:

• Command groovyw module get .
  • Expected result: This should attempt to obtain the "." module, which does not exist.
• Command groovyw module get (
  • Expected result: This should attempt to obtain the "(" module, which does not exist.
• Command groovyw module get )
  • Expected result: This should attempt to obtain the ")" module, which does not exist.
• Command groovyw module get []
  • Expected result: This should attempt to obtain the "[]" module, which does not exist.
• Command groovyw module get *
  • Expected result: This character is specially handled, so all the available modules under the @DestinationSol organisation should be obtained.

Testing multiple wildcards in succession:

• Command groovyw module get ??????
  • Expected result: This should attempt to obtain the formic, slimey and vulcan modules.
• Command groovyw module get ******
  • Expected result: All the available modules under the @DestinationSol organisation should be obtained.

Outstanding Work

• Such a large change needs significant testing. This change needs to be tested extensively to be proven useable, although I think that it should work most of the time. I still need to finish doing this, as I have not fully tested it yet.
  • Further tests to perform:
    • Test that the regex characters ([, ], (, ), *, ., \) are not injected into the code as regexes
    • Test if multiple wildcard selectors can be used to obtain completely different modules, e.g. using the command groovyw module get *de* fo*, which should obtain both the deepspace, federal modules, as well as the deepspace and the formic module.

Notes

This should not break any existing functionality, as the new wildcard handling code is only used when the * or ? symbols are found.

The wildcard strings will have to be escaped using single quotes (e.g. ./groovyw module get '*' rather than ./groovyw module get *) on POSIX platforms (Linux and MacOS), as otherwise the terminal appears to automatically expand them (replacing the arguments with the names all the files in the current directory matching that descriptor) before passing them to the program. I have modified the Windows script to automatically escape the arguments, although I am not sure if it is possible with POSIX platforms to do so.

[GooeyHub]

Can one of the admins please verify this patch?

[Steampunkery]

Hey, looks nice frome here--haven't tested, just glanced at the code. Can you rebase onto develop for me? There were some groovy-related module fetching PRs merged between when you started this branch and now. I tried locally, there shouldn't be any rebase conflicts.

[BenjaminAmos]

The commits should now be rebased onto develop.

[arpitkamboj]

So I've tested this extensively and all the features work as intended but there is one small problem:

The ? wildcard only functions correctly when it is enclosed between two non-wildcard characters

For example:
groovyw module get ? does the same thing as groovyw module get *
groovyw module get s?????? downloads the salvage module but so does groovyw module get s?
*n and ?n both correspond to modules ending with n
ca???on is working as intended and retrieves caution and c?n does not retrieve anything which is good.

Please fix this issue. Everything else is working as intended. Nice Work!

[BenjaminAmos]

The issue should be fixed now. Thank you for testing this. I believe, after further testing myself, that with this change, the wildcards should work now in most user-supplied cases.

[arpitkamboj]

Hey!
So the issue with the ? wildcard is now fixed but there is another weird thing.
I don't know if this is really important though but still:
groovyw module get ?????? tries to get the modules with the file names in the local directory having 6 characters (eg config, engine, gradle) instead of getting modules having names with 6 characters

Please try if you can fix this issue
Thanks for the great work!

[Adrijaned]

Hey, @arpitkamboj , I'm afraid the issue you are describing arises from sh itself, as it replaces the ?s on terminal with the matching local files itself, and does not even pass the ?s to the script. For fixing this, you must simply surround the ?s with ".
@BenjaminAmos I'm afraid you won't be able to fix this no matter how you would try

[BenjaminAmos]

This issue should be fixed on Windows, for the arguments are preserved as-is and I just needed to escape them before passing them as arguments to the java executable. I was doing the same for the "*" character anyway. I can't do anything about the sh script though, as the parameter expansion seems to happen before the arguments are passed to the script.

Thanks for the advice @Adrijaned. I have added a note about this to my original message and added test-cases there so that I am not caught out by that issue again.

[vampcat]

@GooeyHub : add to whitelist

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[Cervator]

Bump - I've reviewed this on the Terasology side and wanted to refer to the comments from here since they likely relate on both sides. Somewhat concerned about the OS complexities introduced by the wildcarding on command line. MovingBlocks/Terasology#3531

Otherwise agreed this is very good work, especially the thorough testing, so thank you so much for that :-)

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[Cervator]

Update: I have added to this for Terasology and tested it out fairly thoroughly. We should be able to merge this here then override with the related files from there, double check / lightly test, then merge.

Was aiming to do that now but I hit #414 on my main test system while out of town. Should be easy to do later or for somebody else. See MovingBlocks/Terasology#3531 for details along with #3679 which I also merged and MovingBlocks/Terasology@dbc32f0 which I added on top.

[Adrijaned]

Works, further review has been done prior on the Terasology PR, merging