Modify ↓

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9027 closed defect (fixed)

[patch] Error in Internal Preset

Reported by: andygol Owned by: Don-vip
Priority: normal Milestone:
Component: Internal preset Version: latest
Keywords: Cc:

Description (last modified by andygol)

I tried to add some additional presets to my preferences and got error message that internal presets have error (see attach)

Attachments (4)

internal_preset_err.png​ (31.3 KB) - added by andygol 6 years ago.
9027.patch​ (715 bytes) - added by AlfonZ 6 years ago.
9027-prevention.patch​ (1.8 KB) - added by AlfonZ 6 years ago.
validate default presets internally in unit tests and externally in build.xml
josm_9027_v2.diff​ (859 bytes) - added by skyper 6 years ago.
patch version 2

Download all attachments as: .zip

Change History (20)

Changed 6 years ago by andygol

Attachment: internal_preset_err.png​ added

comment:1 Changed 6 years ago by andygol

Description: modified (diff)

Changed 6 years ago by AlfonZ

Attachment: 9027.patch​ added

comment:2 Changed 6 years ago by AlfonZ

Summary: Error in Internal Preset → [patch] Error in Internal Preset

Caused by r6195.

comment:3 Changed 6 years ago by Don-vip

Sorry about that :( We need to update build.xml to make it fail in case of a validation error of presets file. It is possible with ​SchemaValidate.

comment:4 in reply to:  2 ; Changed 6 years ago by skyper

Replying to AlfonZ:

Caused by r6195.

Problem is that key does not support values_context but we need it. Simply removing the option does not help with translation (#9010).

comment:5 in reply to:  4 ; Changed 6 years ago by AlfonZ

Replying to skyper:

Replying to AlfonZ:

Caused by r6195.

Problem is that key does not support values_context but we need it. Simply removing the option does not help with translation (#9010).

As far as I know, key doesn't display anything to the user in the preset dialog, it directly sets the tag.

Changed 6 years ago by AlfonZ

Attachment: 9027-prevention.patch​ added

validate default presets internally in unit tests and externally in build.xml

comment:6 in reply to:  3 Changed 6 years ago by AlfonZ

Replying to Don-vip:

Sorry about that :( We need to update build.xml to make it fail in case of a validation error of presets file. It is possible with ​SchemaValidate.

I've attached 9027-prevention.patch​ that checks that.

comment:7 in reply to:  5 ; Changed 6 years ago by skyper

Replying to AlfonZ:

As far as I know, key doesn't display anything to the user in the preset dialog, it directly sets the tag.

Yes, sorry, you are right.

Well, than the problem is the label above which needs text_context.

Changed 6 years ago by skyper

Attachment: josm_9027_v2.diff​ added

patch version 2

comment:8 in reply to:  7 ; Changed 6 years ago by skyper

Replying to skyper:

Replying to AlfonZ:

As far as I know, key doesn't display anything to the user in the preset dialog, it directly sets the tag.

Yes, sorry, you are right.

Well, than the problem is the label above which needs text_context.

Find attached patch​ which fixes the issue.

comment:9 in reply to:  8 ; Changed 6 years ago by AlfonZ

Replying to skyper:

Replying to skyper:

Replying to AlfonZ:

As far as I know, key doesn't display anything to the user in the preset dialog, it directly sets the tag.

Yes, sorry, you are right.

Well, than the problem is the label above which needs text_context.

Find attached patch​ which fixes the issue.

Looking at other items having text_context on label - e.g. (railway) Station​, I wonder if there should be name_context on item as well.

comment:10 in reply to:  9 Changed 6 years ago by skyper

Replying to AlfonZ:

Replying to skyper:

Replying to skyper:

Replying to AlfonZ:

As far as I know, key doesn't display anything to the user in the preset dialog, it directly sets the tag.

Yes, sorry, you are right.

Well, than the problem is the label above which needs text_context.

Find attached patch​ which fixes the issue.

Looking at other items having text_context on label - e.g. (railway) Station​, I wonder if there should be name_context on item as well.

Does translation work case-sensitive ?
In general you are right, but I have to admit that I was too fast and your first patch​ should be sufficient as for labels the whole text is translated and I only find "Skating" at one place in the whole file.

For station you are right that there are some *_context missing.

comment:11 Changed 6 years ago by Don-vip

Owner: changed from team to Don-vip
Status: new → assigned

comment:12 Changed 6 years ago by Don-vip

Resolution: → fixed
Status: assigned → closed

In 6208/josm:

fix #9027 - fix error in internal preset introduced in r6195 (patch by skyper) + validation check in build.xml and junit test (patch by AlfonZ)

comment:13 Changed 6 years ago by Don-vip

Thanks a lot guys !

comment:14 in reply to:  12 Changed 6 years ago by skyper

Replying to Don-vip:

In 6208/josm:

fix #9027 - fix error in internal preset introduced in r6195 (patch by skyper) + validation check in build.xml and junit test (patch by AlfonZ)

Why did you take my patch ? I think AlfonZ's would have been OK.

EDT: I did delete the unneeded option in #9029.

Last edited 6 years ago by skyper (previous) (diff)

comment:15 Changed 6 years ago by Don-vip

Because it was late and I didn't notice. Ok for taking care of that in #9029 :)

comment:16 in reply to:  15 Changed 6 years ago by skyper

Replying to Don-vip:

Because it was late and I didn't notice. Ok for taking care of that in #9029 :)

Alright, thought I missed something. Luckily, no one is perfect.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted.

Add Comment

E-mail address and name can be saved in the Preferences.

Attachments ↑ Description ↑
 
Note: See TracTickets for help on using tickets.