[mkgmap-dev] [mkgmap-svn] Commit r4810: revert changes from r4809 for now, they caused more trouble

Fri Nov 5 10:08:45 GMT 2021

Hi Gerd

This was intended to be a minimal patch. Where there didn't seem to be
a need to replace .equals() with collator.compare() I didn't. Some of
these usages are setting a flag where a name repeats and MdrCheck
assumes exact equals and device behaviour is more difficult to
determine. Others are tests for Region or Country which aren't a
problem.

Ticker

On Thu, 2021-11-04 at 16:57 +0000, Gerd Petermann wrote:
> Hi Ticker,
> 
> OK to fix de-dup where it's wrong. So far I found no problem with the
> different spelling of "De Wijk".
> I don't fully understand the discrepancies in the existing code, but
> the patched code also looks wrong to me with the mixed usage of
> strengths and String.equals() and collator.compare() or
> key.compareTo()
> 
> If I got that right the changes in mdr5 and mdr7 are just refactoring
> and OK.
> Mdr25 could better use Mdr5Record.isSameByName() (although the method
> name is a bit confusing)
> Mdr29 should probably use strength TERTIARY for the collator instead
> of SECONDARY, or there should be a comment why a different strength
> is needed.
> 
> Gerd
> 
> ________________________________________
> Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im Auftrag
> von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> Gesendet: Donnerstag, 4. November 2021 17:17
> An: Development list for mkgmap
> Betreff: Re: [mkgmap-dev] [mkgmap-svn] Commit r4810: revert changes
> from r4809 for now, they caused more trouble
> 
> Hi Gerd
> 
> Although the Sort change fixes the Unicode problem reported by Carlos
> it demonstrated that the different methods of dedupe between Mdr5 and
> Mdr25 can lead to the same error with certain patterns of city names
> and so I really think this should be fixed at the same time.
> 
> If not, it is a problem waiting to happen and the use of, say, code-
> page=936 as in Carlos's second problem, could trigger it.
> 
> Mdr25 could be reverted to have the weird logic, but I think it was a
> bug that would never happen and the code would always confuse.
> 
> The obsolete sort/Mdr20Posn/Index can be fixed as a trivial change
> soon.
> 
> What to do with city case-variants and their street attachments is
> much
> more complicated and needs addressing in the tile-generation phase
> more
> than the MDR generation. This should definitely wait.
> 
> Ticker
> 
> On Thu, 2021-11-04 at 14:10 +0000, Gerd Petermann wrote:
> > Hi Ticker,
> > 
> > My understanding of mdrUnicode_v8.patch is again that only the
> > changes in class Sort are relevant for the unicode error reported
> > by
> > Carlos.
> > Why should we do more right now?
> > 
> > In a second step we can try to simplify the code or fix other
> > issues
> > that came up last weeks like weird code in Mdr25 or the obsolete
> > sort
> > for mdr20.
> > 
> > Gerd
> > 
> > 
> > 
> > 
> > ________________________________________
> > Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im Auftrag
> > von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> > Gesendet: Donnerstag, 4. November 2021 14:34
> > An: Development list for mkgmap
> > Betreff: Re: [mkgmap-dev] [mkgmap-svn] Commit r4810: revert changes
> > from r4809 for now, they caused more trouble
> > 
> > Hi Gerd
> > 
> > This was a bit arbitrary and maybe I should have reverted it to
> > .equals().
> > 
> > Generally Regions and Countries don't cause a problem because they
> > (almost always) originate from --bounds and go through
> > PlacesFile.createRegion() / createCountry() which stops any case
> > difference within a tile.
> > 
> > Ticker
> > 
> > On Thu, 2021-11-04 at 13:15 +0000, Gerd Petermann wrote:
> > > Hi Ticker,
> > > 
> > > why collator.setStrength(Collator.SECONDARY); in Mdr29?
> > > 
> > > Gerd
> > > 
> > > ________________________________________
> > > Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im
> > > Auftrag
> > > von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> > > Gesendet: Donnerstag, 4. November 2021 12:40
> > > An: Development list for mkgmap
> > > Betreff: Re: [mkgmap-dev] [mkgmap-svn] Commit r4810: revert
> > > changes
> > > from r4809 for now, they caused more trouble
> > > 
> > > Hi Gerd
> > > 
> > > Yes, patch r4809 caused the crash with Arndt's data (--lower-case
> > > and
> > > differently cased city names).
> > > 
> > > Ticker
> > > 
> > > On Thu, 2021-11-04 at 11:11 +0000, Gerd Petermann wrote:
> > > > Hi Ticker,
> > > > 
> > > > thanks, will have a closer later. Just to make sure:
> > > > My understanding is that the assertion with Arndts data was
> > > > caused
> > > > by
> > > > your patch (r4809) and everything worked fine with r4808 /
> > > > r4810.
> > > > 
> > > > Gerd
> > > > 
> > > > ________________________________________
> > > > Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im
> > > > Auftrag
> > > > von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> > > > Gesendet: Donnerstag, 4. November 2021 11:55
> > > > An: Development list for mkgmap
> > > > Betreff: Re: [mkgmap-dev] [mkgmap-svn] Commit r4810: revert
> > > > changes
> > > > from r4809 for now, they caused more trouble
> > > > 
> > > > Hi Gerd
> > > > 
> > > > Here is a minimal patch to stop the 2 assertion crashes:
> > > > 1/ unspecified sort for some blocks of Unicode chararacters.
> > > > 2/ inconsistent MDR20 city positions that could result from
> > > > city
> > > > names
> > > >    with different case or use of non-unicode multi-byte
> > > > codepage
> > > > 
> > > > For 2/ with option --lower-case, the behaviour of how and which
> > > > cities
> > > > with capitalisation differences are shown in the find list, and
> > > > which
> > > > streets are attached to them is:
> > > > 1/ device/BaseCamp/MapSource dependant.
> > > > 2/ depends on how the streets are spread over tiles where there
> > > > is
> > > >    possible name conflict.
> > > > 3/ Affected by the presence of city POI on a tile.
> > > > 
> > > > Ticker
> > > > 
> > > > _______________________________________________
> > > > mkgmap-dev mailing list
> > > > mkgmap-dev at lists.mkgmap.org.uk
> > > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > 
> > > 
> > > _______________________________________________
> > > mkgmap-dev mailing list
> > > mkgmap-dev at lists.mkgmap.org.uk
> > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > _______________________________________________
> > > mkgmap-dev mailing list
> > > mkgmap-dev at lists.mkgmap.org.uk
> > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > 
> > 
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev at lists.mkgmap.org.uk
> > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev at lists.mkgmap.org.uk
> > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> 
> 
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev