Fix BC dates in the range index DateConverter #1200

Merged
merged 2 commits into from Dec 14, 2016

Conversation

Projects
None yet
5 participants
@olvidalo
Contributor

olvidalo commented Dec 14, 2016

In our project we have to deal with dates BC, like "-300" or "-0004-03-29" that, when indexed, have to be normalized using the DateConverter (which should definitely be documented somewhere... utter happiness ensued when I stumbled upon this while digging through the sources 馃槃 )

These changes make the converter work correctly with negative (BC) dates. Would be excellent if this could go into 3.0.

@Override
public Field toField(String fieldName, String content) {
try {
DateValue dv;
- if (content.indexOf('-') < 0) {
+ if (content.indexOf('-') < 1) {

This comment has been minimized.

@dizzzz

dizzzz Dec 14, 2016

Member

Please use the String.contains()

@dizzzz

dizzzz Dec 14, 2016

Member

Please use the String.contains()

This comment has been minimized.

@olvidalo

olvidalo Dec 14, 2016

Contributor

You mean like !(content.substring(1).contains('-'))? As this should only evaluate false if the string contains '-' in any other position than the first position, accounting for negative "only-year" values.

Seems more complicated to me, but if you insist... 馃槈

@olvidalo

olvidalo Dec 14, 2016

Contributor

You mean like !(content.substring(1).contains('-'))? As this should only evaluate false if the string contains '-' in any other position than the first position, accounting for negative "only-year" values.

Seems more complicated to me, but if you insist... 馃槈

This comment has been minimized.

@adamretter

adamretter Dec 14, 2016

Member

@olvidalo I think it is fine as it is

@adamretter

adamretter Dec 14, 2016

Member

@olvidalo I think it is fine as it is

@dizzzz

This comment has been minimized.

Show comment
Hide comment
@dizzzz

dizzzz Dec 14, 2016

Member

Nice :) any idea why Travis fails?

Member

dizzzz commented Dec 14, 2016

Nice :) any idea why Travis fails?

@olvidalo

This comment has been minimized.

Show comment
Hide comment
@olvidalo

olvidalo Dec 14, 2016

Contributor

Not sure! It seems to fail due to dependency issues regarding betterform, it cannot find jgroup.

Maybe just a glitch? Can't imagine that this is caused by these changes somehow...

Contributor

olvidalo commented Dec 14, 2016

Not sure! It seems to fail due to dependency issues regarding betterform, it cannot find jgroup.

Maybe just a glitch? Can't imagine that this is caused by these changes somehow...

@olvidalo olvidalo closed this Dec 14, 2016

@olvidalo olvidalo deleted the olvidalo:feature/range-dateconverter-fix-bc-dates branch Dec 14, 2016

@olvidalo olvidalo restored the olvidalo:feature/range-dateconverter-fix-bc-dates branch Dec 14, 2016

@olvidalo olvidalo reopened this Dec 14, 2016

@joewiz

This comment has been minimized.

Show comment
Hide comment
@joewiz

joewiz Dec 14, 2016

Member

Nice addition! Thanks!

Member

joewiz commented Dec 14, 2016

Nice addition! Thanks!

@adamretter adamretter merged commit 0d88592 into eXist-db:develop Dec 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@adamretter

This comment has been minimized.

Show comment
Hide comment
@adamretter

adamretter Dec 14, 2016

Member

@olvidalo Thanks for your contribution :-)

Member

adamretter commented Dec 14, 2016

@olvidalo Thanks for your contribution :-)

@olvidalo

This comment has been minimized.

Show comment
Hide comment
@olvidalo

olvidalo Dec 14, 2016

Contributor

My pleasure. Thanks for merging 馃帀

Contributor

olvidalo commented Dec 14, 2016

My pleasure. Thanks for merging 馃帀

@duncdrum

This comment has been minimized.

Show comment
Hide comment
@duncdrum

duncdrum Dec 14, 2016

Member

just out of curiosity @olvidalo will this normalise '0' to '-0001'?

Member

duncdrum commented Dec 14, 2016

just out of curiosity @olvidalo will this normalise '0' to '-0001'?

@olvidalo

This comment has been minimized.

Show comment
Hide comment
@olvidalo

olvidalo Dec 14, 2016

Contributor

No, it would "normalise" it to "0000-01-01" which would then throw an error somewhere down the line because this is an invalid value for xs:date.

Contributor

olvidalo commented Dec 14, 2016

No, it would "normalise" it to "0000-01-01" which would then throw an error somewhere down the line because this is an invalid value for xs:date.

@dizzzz

This comment has been minimized.

Show comment
Hide comment
@dizzzz

dizzzz Dec 15, 2016

Member

Thnx!

Member

dizzzz commented Dec 15, 2016

Thnx!

@dizzzz

This comment has been minimized.

Show comment
Hide comment
@dizzzz

dizzzz Dec 15, 2016

Member

Thnx!

Member

dizzzz commented Dec 15, 2016

Thnx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment