search

GrandOrgue / GoOdf

A tool for creating/editing organ definition files for GrandOrgue
GNU General Public License v3.0
12 stars 2 forks source link

Set TextBreakWidth=0 on load where DispLabelText is set empty #74

Closed ahall41 closed 1 year ago

ahall41 commented 1 year ago

If DispLabelText is empty (i.e. the label is in the image), this needs to be included in the generated PanelElement, otherwise the name of the parent control "bleeds" through (especially so as it is red by default).

larspalo commented 1 year ago

No, it's meant that one should use TextBreakWidth=0 to not write text. The DispLabelText is never ment to be empty, it's used to display a shortened (not none) version of the label. (this is obvious if you would read the GO help)

ahall41 commented 1 year ago

In that case ... when converting old style panels to new ... its not setting TextBreakWidth to 0! FYI I was making a few changes to Friesach and (nice touch to do the conversion), this is what happened! When converting a working old format file - it shouldn't change things unexpectedly? TextBreakWidth=0 sounds like a (somewhat obscure) 'feature' to me - DispLabelText={null} is far more logical - if it's never meant to be empty then maybe it should flag up as a warning on load?

larspalo commented 1 year ago

if it's never meant to be empty then maybe it should flag up as a warning on load?

Yes, I even consider it as a bug in GO (that DispLabelText can be set without a value when it's supposed to be used in quite another way). But there are so many more important things to do. TextBreakWidth=0 is the canonical way to hide the written text. This is safe as it should be secure for the future.

This is clear in the help file and was obviously the way we did it "back then"...

TextBreakWidth
(integer 0 - text rectangle width, default: TextWidth) If 0, no text is displayed. Otherwise the value specifies the maximum line width used for text breaking.

The problem is that for a long time many new users of GO didn't get the guidance they would have needed to understand how things were "really supposed" to be done in the organ description file. The fact that something "can" be done as a side effect of a bug is not equal to that it's an acceptable way even if it works now - the implementation can change (the bug might be fixed) in the future and the mis-use of a feature won't work as before.

larspalo commented 1 year ago

@ahall41 But you're maybe right, I could check for an empty value of DispLabelText and convert it to a TextBreakWidth of 0 in GOODF when reading/importing an .organ file. I'll look into it!

ahall41 commented 1 year ago

That would be very good - I've changed my own scripts to reflect this. A side effect of copying someone else's ODF rather than (as the usual last resort) read the manual in its entirety!

larspalo commented 1 year ago

Should be fixed with c00ed247ecc8955ac43e06f6f8b3286adeb8a74e.

ahall41 commented 1 year ago

Yes, thank you. Title changed to reflect the actual issue.