Re: Arrow of NSMenu

[Fred Kiefer]

Hi Eric,

thank you! This new change looks a lot cleaner and leaves all the mapping logic 
inside of NSImage. I think it will be a lot easier to maintain.

Fred

On the road

Am 06.04.2013 um 21:51 schrieb Eric Wasylishen <address@hidden>:

> Hi Fred,
> 
> On 2013-04-06, at 9:03 AM, Fred Kiefer <address@hidden> wrote:
> 
>> Hi Eric,
>> 
>> I don't like this change very much and will try explain why. This does not 
>> mean that I doubt the technical correctness of this patch. I just think we 
>> should try to find a better solution.
> 
> Agreed, it's ugly.
> 
>> - First off, I don't really see the issue here. This may be because I don't 
>> use themes. But can somebody please explain what would be the problem with 
>> using the official names for images in themes? Riccardo already stated that 
>> things would work when using the name NSMenuArrow.
> 
> If we don't use the nsmappings.strings for themes, themes may have to provide 
> a lot of duplicate files (e..g common_3DArrowRight.tiff, NSMenuArrow.tiff) 
> with the same contents. Not a huge problem… but it's ugly to have different 
> image lookup logic for images inside themes and other images.
> 
>> - The big doubt I am having with the change is that now GSTheme has to know 
>> about that name mapping which was internal to NSImage up to now.
>> A solution where similar code would be used inside the NSImage method 
>> _setImagePath:name: seems a lot cleaner to me. If we build up that reverse 
>> map you are using, all the necessary information should be available in that 
>> method. I think this would belong into the else case of the if (image != 
>> nil) test you introduced.
> 
> I considered that - so +[NSImage _setImagePath:name:] for common_3DArrowRight 
> would also set the path for NSMenuArrow and anything else that maps to 
> common_3DArrowRight.
> 
> There is a corner case with that; if the theme provides both NSMenuArrow and 
> common_3DArrowRight, the image used for NSMenuArrow depends on which call to 
> +[NSImage _setImagePath:name:] is made first.
> 
>> - Another way to get rid of the problem would be to completely remove the 
>> name mapping from NSImage. I am a bit reluctant to propose this. That 
>> mechanism has been around for a very long time and it allows us to have 
>> clearer names. But with the theme code in place this mechanism isn't needed 
>> that much any more.
> 
> I committed a more radical redesign that is much cleaner, I think.
> Pasting my changelog comment:
> 
>    I removed the step in theme activation where we call
>    +[NSImage _setImagePath:name:] on each image in the theme, and instead
>    modified +[NSImage _pathForImageNamed:] to also search the theme images
>    directory.
> 
>    When a GSTheme activates now, it only calls +[NSImage _reloadCachedImages]
>    which checks all NSImage cached by name and reloads any whose path has
>    changed.
> 
> My only worry is whether this will break the GTK or windows themes. IIRC they 
> replace the NSImage class and override +imageNamed:, so I think they'll still 
> work.
> 
> Eric
> 
>> Cheers
>> Fred
>> 
>> On 06.04.2013 10:06, Eric Wasylishen wrote:
>>> Hi Riccardo,
>>> 
>>> I committed a fix in r36474. What I ended up doing is, when a GSTheme 
>>> activates, it takes the image name -> path dictionary of images in the 
>>> theme, and "expands" it by applying all of the nsmappings.strings mappings.
>>> 
>>> So if your theme defines common_3DArrowRight.tiff but not NSMenuArrow, I'll 
>>> produce a dictionary like:
>>> {
>>>    "common_3DArrowRight" : "path/to/common_3DArrowRight.tiff",
>>>    "NSMenuArrow" : "path/to/common_3DArrowRight.tiff",
>>> }
>>> 
>>> This expanded set of images is then applied to the app state using 
>>> +[NSImage _setImagePath:name:], and the same expanded set is unregistered 
>>> when the theme deactivates.
>>> 
>>> Hope this works for you, and the behaviour sounds sensible.
>>> 
>>> Cheers.
>>> Eric
>>> 
>>> On 2013-04-05, at 5:25 AM, Riccardo Mottola <address@hidden> wrote:
>>> 
>>>> Hi all,
>>>> 
>>>> On 03/28/13 16:40, Eric Wasylishen wrote:
>>>>> 
>>>>> Hey Riccardo, check the nsmappings.strings file in Images. I think that 
>>>>> maps nsmenuarrow to one of the common_ images.
>>>>> 
>>>> back to the original problem, which is different from what German supposed.
>>>> 
>>>> Let's remember that we have a mapping
>>>> 
>>>> NSMenuArrow = common_3DArrowRight;
>>>> 
>>>> 
>>>> Leaving out Thematic for a moment, I found that placing inside the Theme 
>>>> Images a an image named
>>>> 
>>>> common_3DArrowRight.tif
>>>> 
>>>> doesn't work, while putting one called
>>>> 
>>>> NSMenuArrow.tif
>>>> 
>>>> works fine and the image gets loaded even dynamically when changing the 
>>>> theme.
>> 
>> 
>> _______________________________________________
>> Gnustep-dev mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/gnustep-dev
>