Re: Arrow of NSMenu
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 :
> Hi Fred,
>
> On 2013-04-06, at 9:03 AM, Fred Kiefer 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
>>> 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
>> Gnustep-dev@gnu.org
>> https://lists.gnu.org/mailman/listinfo/gnustep-dev
>
___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: Arrow of NSMenu
Hi Fred,
On 2013-04-06, at 9:03 AM, Fred Kiefer 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
>> 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
> Gnustep-dev@gnu.org
> https://lists.gnu.org/mailman/listinfo/gnustep-dev
___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: Arrow of NSMenu
Hi, I am a little in doubt here, I think Fred has some valid points. Fred Kiefer 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. - 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. Well, but if we have a remapping it should work. IE. our actual file is common_3DArrowRight.tif in the standard theme, why shouldn't that be true also in the theme-supplied 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. - 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. Well, mapping works with the "default theme". I too don't like that GSTheme shouldn't know about the remapping. You ask for NSArrow, the mapping converts it to another name, now this other named image could be in the default theme or in a supplied theme. I actually do not understand where the mechamism breaks. Riccardo ___ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: Arrow of NSMenu
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.
- 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.
- 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.
- 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.
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 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
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: Arrow of NSMenu
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 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.
>
>
> Riccardo
___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
