cursor position tool should consider locale when parsing coord string

[nprigour]

Setting the coordinate in the cursor position textbox seems to be problematic when locale used is one that uses comma (,) separator instead of dot (.)
before this patch --> based on the locale of the application the format displayed could be either:
xxxx.xxxx, yyyy.yyyy (i.e. US locale) or xxxx,xxxx, yyyy,yyyy (i.e. German or France locale). In the latter case when trying to center the viewport around the coordinate the coord coefficient parsing is problematic.
Afte the patch --> based on the locale of the application the format displayed could be either:
xxxx.xxxx yyyy.yyyy (i.e. US locale) or xxxx,xxxx yyyy,yyyy (i.e. German or France locale). Parsing and viewport setting does not suffer from the issue of multiple commas and works correct

Signed-off-by: Nikolaos Pringouris nprigour@gmail.com

[fgdrf]

Great, thanks for this fix

Any chance to implement tests for these scenarios. would help to assure for future refactorings/changes.

[nprigour]

The problem with adding a test for this PR is that in order to verify the behavior of the tool we need the udig application with a map loaded and the ability to input coordinates in the cursor position line item (see below) in order to verify proper setting of viewport.

Is this somehow possible with the present test framework?

[fgdrf]

I guess CursorPositionTest implemented such tests already and I'm wondering if we could make it parametrized

Also I'm not sure if test would fail if it runs different locale settings. in static CursorPosition.parse() method is a similiar implementation with comma and space. Maybe its worth to refactor class under test

[nprigour]

Hi @fgdrf ,

I proceed with another commit which provides a refactored implementation of CursorPositionTest that checks parsing using 2 different Locales (one where decimal separator in . and another one with ,)

Furthermore I proceed with further enhancements in CursorPosition so that:

1. Locale setting are correctly retrieved during parse() and getString() method
2. parsing of String to coordinate now utilizes StringUtils which provides for better handling of split process, trimming, removal of undesired characters and handling of consecutive characters (i.e multiple spaces between coord coefficients)
3. dot (.) as decimal separator can now be used always independent of Locale when setting coordinates. On the contrary comma (,) as decimal separator may apply only to specific locales (i.e. FR, DE, GR etc)
4. comma (except for Locales like FR, DE etc and in the case there are decimal digits) or space may be used as coordinate coefficient separators
5. getString(Coordinate coord) method moved outside from inner class and become public and static so that it can be available by other classes that may need to have a common way to transform Coordinates to String

P.S: the formatting of the CursorPosition class is awful. Maybe it is worth after reviewing these commits and before closing the PR to apply style formatter to the whole class.

[nprigour]

Hi @fgdrf ,
I added the locale rule you suggested and split tests to 2 different classes based on applied Locale

[fgdrf]

I guess regarding Eclipse IP its not allowed to copy code from other projects. So it can inspire uDig but this class needs to be re-written or we add a dependency to the project to use it as a library.

[nprigour]

The class is quite elementary in terms of behavior and applied logic. Would it be OK if I just rename it (for example ConfigureLocaleRule) and possibly alter a couple of Comments

[nprigour]

Latest commit addresses:

• refactor of test code to avoid eclipse IP issues
• addition of header comment in newly created files
• fix of an erroneous if check introduced during previous commit in CursorPosition.parse(...) method

[fgdrf]

CI-BUILD : https://ci.eclipse.org/udig/job/uDig-PR/35/

[nprigour]

Hmm the failure seems to be related to the naming convention of the concrete test classes which should be in the the form XXXXXTest.java otherwise the class is not recognized as a junit test from maven. I rename them so now the test should be OK. Please re-run jenkins CI

[fgdrf]

here we go : https://ci.eclipse.org/udig/job/uDig-PR/36/

[nprigour]

@fgdrf ,

If jenkins job executes without errors I would strongly suggest before you merge the PR to let me push another commit for fixing formatting in CursorPosition class which as I mentioned in a previous comment is "awful".

[fgdrf]

Job feedback looks good, waiting for your re-formatings

[nprigour]

I just apply udig formatting to CursorPosition class. I also slightly reformat the modified test classes

[fgdrf]

https://ci.eclipse.org/udig/job/uDig-PR/37/ was fine. Thanks for contributing!