Lucene - Core
  1. Lucene - Core
  2. LUCENE-2643

StringHelper#stringDifference is wrong about supplementary chars

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 3.0, 3.0.1, 3.0.2
    • Fix Version/s: 3.1, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      StringHelper#stringDifference does not take supplementary characters into account. Since this is not used internally at all we should think about removing it but I guess since it is not too complex we should just or fix it for bwcompat reasons. For released versions we should really fix it since folks might use it though. For trunk we could just drop it.

      1. LUCENE-2643.patch
        4 kB
        Simon Willnauer
      2. LUCENE-2643.patch
        0.7 kB
        Robert Muir
      3. LUCENE-2643.patch
        2 kB
        Simon Willnauer

        Activity

        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1
        Hide
        Simon Willnauer added a comment -

        Committed revision 1043738.

        Show
        Simon Willnauer added a comment - Committed revision 1043738.
        Hide
        Simon Willnauer added a comment -

        here is a patch against 3x - I will commit shortly

        Show
        Simon Willnauer added a comment - here is a patch against 3x - I will commit shortly
        Hide
        Simon Willnauer added a comment -

        And we should drop it in 3.x and 4.0.

        since this it gone from 4.0 I will remove from 3.x now too.

        Show
        Simon Willnauer added a comment - And we should drop it in 3.x and 4.0. since this it gone from 4.0 I will remove from 3.x now too.
        Hide
        Robert Muir added a comment -

        moving out

        Show
        Robert Muir added a comment - moving out
        Hide
        Michael McCandless added a comment -

        Lets drop it from 4.0 and 3.x and deprecate in the 3.0 branch so if we do a 3.0.3 release one day people get a heads up.

        I think doing anything in 3.0.x is optional here... above & beyond the call of duty

        And we should drop it in 3.x and 4.0.

        Show
        Michael McCandless added a comment - Lets drop it from 4.0 and 3.x and deprecate in the 3.0 branch so if we do a 3.0.3 release one day people get a heads up. I think doing anything in 3.0.x is optional here... above & beyond the call of duty And we should drop it in 3.x and 4.0.
        Hide
        Simon Willnauer added a comment -

        Lets drop it from 4.0 and 3.x and deprecate in the 3.0 branch so if we do a 3.0.3 release one day people get a heads up.

        thoughts?

        Show
        Simon Willnauer added a comment - Lets drop it from 4.0 and 3.x and deprecate in the 3.0 branch so if we do a 3.0.3 release one day people get a heads up. thoughts?
        Hide
        Robert Muir added a comment -

        +1 to at least removing from 4.0.

        Show
        Robert Muir added a comment - +1 to at least removing from 4.0.
        Hide
        Michael McCandless added a comment -

        Why not drop it from 3.x/3.0.x as well?

        It's an internal API so there's no need to deprecate it first?

        Show
        Michael McCandless added a comment - Why not drop it from 3.x/3.0.x as well? It's an internal API so there's no need to deprecate it first?
        Hide
        Simon Willnauer added a comment -

        I also merged into 3_x - should we backport to 3.0.x too?

        simon

        Show
        Simon Willnauer added a comment - I also merged into 3_x - should we backport to 3.0.x too? simon
        Hide
        Simon Willnauer added a comment -

        patch to deprecate (we should just remove from trunk though!)

        +1 - I committed that in 1024128

        thanks robert

        Show
        Simon Willnauer added a comment - patch to deprecate (we should just remove from trunk though!) +1 - I committed that in 1024128 thanks robert
        Hide
        Robert Muir added a comment -

        patch to deprecate (we should just remove from trunk though!)

        Show
        Robert Muir added a comment - patch to deprecate (we should just remove from trunk though!)
        Hide
        Simon Willnauer added a comment -

        drop in trunk and mark deprecated in 3.x?

        yeah that makes sense... shall we fix it in 3.x (fix means support suppl. chars)? I don't have strong feelings about it.

        Show
        Simon Willnauer added a comment - drop in trunk and mark deprecated in 3.x? yeah that makes sense... shall we fix it in 3.x (fix means support suppl. chars)? I don't have strong feelings about it.
        Hide
        Robert Muir added a comment -

        drop in trunk and mark deprecated in 3.x?

        regardless of whether its right or wrong, if we arent using it, i think its good to clean house.

        Show
        Robert Muir added a comment - drop in trunk and mark deprecated in 3.x? regardless of whether its right or wrong, if we arent using it, i think its good to clean house.
        Hide
        Simon Willnauer added a comment -

        since its unused, its not obvious that its "wrong" (its correct if you want the first code unit difference)

        yeah - my interpretation would be its wrong since you use Character.charAt(int) with the index of the first code unit. anyway - we should drop for trunk but I am not sure if we should for 3.x. I mean this is not that much of a deal anyway.

        Show
        Simon Willnauer added a comment - since its unused, its not obvious that its "wrong" (its correct if you want the first code unit difference) yeah - my interpretation would be its wrong since you use Character.charAt(int) with the index of the first code unit. anyway - we should drop for trunk but I am not sure if we should for 3.x. I mean this is not that much of a deal anyway.
        Hide
        Robert Muir added a comment -

        My vote would be to drop it if we arent using it, its @lucene.internal.

        since its unused, its not obvious that its "wrong" (its correct if you want the first code unit difference)

        Show
        Robert Muir added a comment - My vote would be to drop it if we arent using it, its @lucene.internal. since its unused, its not obvious that its "wrong" (its correct if you want the first code unit difference)
        Hide
        Simon Willnauer added a comment -

        here is a patch

        Show
        Simon Willnauer added a comment - here is a patch

          People

          • Assignee:
            Simon Willnauer
            Reporter:
            Simon Willnauer
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development