Modify ↓

Opened 10 years ago

Closed 10 years ago

Last modified 10 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 10 years ago.
9027.patch​ (715 bytes) - added by AlfonZ 10 years ago.
9027-prevention.patch​ (1.8 KB) - added by AlfonZ 10 years ago.
validate default presets internally in unit tests and externally in build.xml
josm_9027_v2.diff​ (859 bytes) - added by skyper 10 years ago.
patch version 2

Download all attachments as: .zip

Change History (20)

Changed 10 years ago by andygol

Attachment: internal_preset_err.png​ added

comment:1 Changed 10 years ago by andygol

Description: modified (diff)

Changed 10 years ago by AlfonZ

Attachment: 9027.patch​ added

comment:2 Changed 10 years ago by AlfonZ

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

Caused by r6195.

comment:3 Changed 10 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 10 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 10 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 10 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 10 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 10 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 10 years ago by skyper

Attachment: josm_9027_v2.diff​ added

patch version 2

comment:8 in reply to:  7 ; Changed 10 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 10 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 10 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 10 years ago by Don-vip

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

comment:12 Changed 10 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 10 years ago by Don-vip

Thanks a lot guys !

comment:14 in reply to:  12 Changed 10 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 10 years ago by skyper (previous) (diff)

comment:15 Changed 10 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 10 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.