issue with auto-enable of Menus

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

issue with auto-enable of Menus

Riccardo Mottola-5
Hi,

I enhanced Graphos to enable/disable automatically menus dependent on
the selection of objects.
E.g. the idea is that you can edit something only if an object is
selected.

I detect the menu depending on the connected action.

The code works for certain custom actions, but not for cut/copy/delete

This works fine on Mac, but not on GNUstep!
I wonder why?

   if (action == @selector(copy:) || action == @selector(cut:) ||
action == @selector(delete:))
     {
       if (selectedObjs)
         return YES;
       else
         return NO;
     }

the relevant code is in GRDocView.m

http://svn.savannah.gnu.org/viewvc/gap/trunk/user-apps/Graphos/GRDocView.m?revision=3565&view=markup

Riccardo

--
Proudly sent with GNUMail !


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Fred Kiefer


> Am 12.11.2017 um 21:16 schrieb Riccardo Mottola <[hidden email]>:
>
> I enhanced Graphos to enable/disable automatically menus dependent on the selection of objects.
> E.g. the idea is that you can edit something only if an object is selected.
>
> I detect the menu depending on the connected action.
>
> The code works for certain custom actions, but not for cut/copy/delete
>
> This works fine on Mac, but not on GNUstep!
> I wonder why?
>
>  if (action == @selector(copy:) || action == @selector(cut:) || action == @selector(delete:))
>    {
>      if (selectedObjs)
>        return YES;
>      else
>        return NO;
>    }
>
> the relevant code is in GRDocView.m
>
> http://svn.savannah.gnu.org/viewvc/gap/trunk/user-apps/Graphos/GRDocView.m?revision=3565&view=markup

Could you please provide some additional information about this code. If you add a bit of logging you should be able to report all this:

- Is the code called at all? This could be caused by a bug in my menu handling change from last year.
- What is the value of selectedObjs? Does this match your expectations?
- Which compiler and runtime are you using? With gcc an the old GNUstep runtime this code cannot work at all. There you will have to use sel_isEqual()

Hope this helps,
Fred

> --
> Proudly sent with GNUMail !

Great to hear!
_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Wolfgang Lux
In reply to this post by Riccardo Mottola-5
Hi Riccardo,

>  if (action == @selector(copy:) || action == @selector(cut:) || action == @selector(delete:))

you must use sel_isEqual to compare selectors in portable code, i.e.,
  if (sel_isEqual(action, @selector(copy:)) || sel_isEqual(action, @selector(cut:)) || sel_isEqual(action, @selector(delete:))
While sel_isEqual is just an alias for == on Apple's runtime (and the GNUstep runtime, I assume), this need not be the case for other runtimes. In particular, for the old GNU runtime sel_isEqual is not equivalent to ==, as you've observed.

Wolfgang



_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Riccardo Mottola-5
Hi Wolfgang, Fred,

Wolfgang Lux wrote:
>>   if (action == @selector(copy:) || action == @selector(cut:) || action == @selector(delete:))
> you must use sel_isEqual to compare selectors in portable code, i.e.,
>    if (sel_isEqual(action, @selector(copy:)) || sel_isEqual(action, @selector(cut:)) || sel_isEqual(action, @selector(delete:))
> While sel_isEqual is just an alias for == on Apple's runtime (and the GNUstep runtime, I assume), this need not be the case for other runtimes. In particular, for the old GNU runtime sel_isEqual is not equivalent to ==, as you've observed.

that was it! I did run on gcc5 with the gcc runtime.

What was confusing was that "some" menus were enabled, some not! I was
fearing that auto-enable was working for some menus and other not, an
issue in the responder chain, while it was actually much simpler.

Thanks,
Riccardo

PS: this leads to a more usable editing in Graphos: quite nice,
something I had to do years ago!

_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

David Chisnall
In reply to this post by Wolfgang Lux
On 13 Nov 2017, at 08:17, Wolfgang Lux <[hidden email]> wrote:
>
> While sel_isEqual is just an alias for == on Apple's runtime (and the GNUstep runtime, I assume), this need not be the case for other runtimes. In particular, for the old GNU runtime sel_isEqual is not equivalent to ==, as you've observed.

It’s also not == on the GNUstep runtime, and can’t easily be with a new ABI if we want it to work on ELF / COFF platforms.  Apple relies on the run-time linker deduplicating their selector table and inserting selectors into a contiguous region of memory.  

With the GNUstep (or GCC) runtime, == will work if both selectors are defined (i.e. created from @selector, or from a caller setting the _cmd parameter) in the same compilation unit.  With the new ABI that’s I’m (intermittently) working on, it will work in the same DSO, but in both cases it’s fragile.

Clang will warn if you do == comparisons on selectors (even on OS X), so I’m surprised that this code has survived the last few years.

PSA: Even if you really like GCC and want to use GCC for all of your shipping code, please make sure that it compiles with clang -Werror before spending ages trying to figure out why it’s broken.  The clang static analyser will also find a load more common bugs in Objective-C.

David


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Ivan Vučica-2
In reply to this post by Riccardo Mottola-5
To add to thread from a different perspective...

Menu items should auto enable/disable depending on whether the target object supports the passed selector.

Would it make sense to not have custom code to enable/disable menu items, and just have appkit do it for you depending on the target?

This works especially magically if you use target NSFirstResponder and target the “first responder” correctly. Depending on how Graphos works, this might be infeasible, of course.

ned, 12. stu 2017. u 20:17 Riccardo Mottola <[hidden email]> napisao je:
Hi,

I enhanced Graphos to enable/disable automatically menus dependent on
the selection of objects.
E.g. the idea is that you can edit something only if an object is
selected.

I detect the menu depending on the connected action.

The code works for certain custom actions, but not for cut/copy/delete

This works fine on Mac, but not on GNUstep!
I wonder why?

   if (action == @selector(copy:) || action == @selector(cut:) ||
action == @selector(delete:))
     {
       if (selectedObjs)
         return YES;
       else
         return NO;
     }

the relevant code is in GRDocView.m

http://svn.savannah.gnu.org/viewvc/gap/trunk/user-apps/Graphos/GRDocView.m?revision=3565&view=markup

Riccardo

--
Proudly sent with GNUMail !


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
--
Sent from Gmail Mobile on iPad

_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Riccardo Mottola-5
Hi,

Ivan Vučica wrote:
>
> Menu items should auto enable/disable depending on whether the target
> object supports the passed selector.
>
> Would it make sense to not have custom code to enable/disable menu
> items, and just have appkit do it for you depending on the target?

auto-enable already works that way. If you attach to NSFirst it goes
down the responder chain.

>
> This works especially magically if you use target NSFirstResponder and
> target the “first responder” correctly. Depending on how Graphos
> works, this might be infeasible, of course.

It works but it is not "enough".

If I want to enable "copy" only if an object is selected I need to ask
my document or view if it has something selected inside. If you just
validate if your controller supports copy there still might be nothing
to do.
I wanted to go more "finegrained", up to the point where certain menus
are enabled only if certain things are selected.

Riccardo

_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Ivan Vučica-2
On Mon, Nov 13, 2017 at 3:03 PM Riccardo Mottola <[hidden email]> wrote:
Hi,

Ivan Vučica wrote:
>
> Menu items should auto enable/disable depending on whether the target
> object supports the passed selector.
>
> Would it make sense to not have custom code to enable/disable menu
> items, and just have appkit do it for you depending on the target?

auto-enable already works that way. If you attach to NSFirst it goes
down the responder chain.

...yes, I know, that's why I was pointing it out? :)
 

>
> This works especially magically if you use target NSFirstResponder and
> target the “first responder” correctly. Depending on how Graphos
> works, this might be infeasible, of course.

It works but it is not "enough".

If I want to enable "copy" only if an object is selected I need to ask
my document or view if it has something selected inside. If you just
validate if your controller supports copy there still might be nothing
to do.
I wanted to go more "finegrained", up to the point where certain menus
are enabled only if certain things are selected.

I'm not sure you picked up on my suggestion :)

Why not add the object that receives these things to the receiver chain?

Or: why not have every selectable object as an NSView subclass?

It could be infeasible given a codebase. But, if I was writing a new project where I wanted the menus to vary, I would do it this way -- it would also allow me to offload some of the 'is this selected?' logic.

Or, if I need multiple selection, I could create a virtual responder or NSView.

_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Wolfgang Lux
In reply to this post by Ivan Vučica-2
> Am 13.11.2017 um 12:54 schrieb Ivan Vučica <[hidden email]>:
>
> To add to thread from a different perspective...
>
> Menu items should auto enable/disable depending on whether the target object supports the passed selector.

I'm afraid this logic is too simple. Yes, the target object should support the selector. But there may be additional conditions: For instance, a paste menu item should be enabled only when the pasteboard contains content that is compatible with the receiver. And cut, copy and delete menu items should be enabled only if the receiver has a non-empty selection.

> Would it make sense to not have custom code to enable/disable menu items, and just have appkit do it for you depending on the target?

In my experience, custom code is unavoidable in almost any non-trivial case because the AppKit is not able to determine exactly when a menu item is applicable and when not. Having support for the selectors is a necessary but not a sufficient condition.

Wolfgang

_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

David Chisnall
On 13 Nov 2017, at 17:52, Wolfgang Lux <[hidden email]> wrote:
>
>> Would it make sense to not have custom code to enable/disable menu items, and just have appkit do it for you depending on the target?
>
> In my experience, custom code is unavoidable in almost any non-trivial case because the AppKit is not able to determine exactly when a menu item is applicable and when not. Having support for the selectors is a necessary but not a sufficient condition.

IOKit improves this relative to AppKit by adding the delegates to the responder chain.  For custom views, I’ve taken to doing this and forwarding some of the responder chain queries to the delegate.  It would also be nice if there were some convention such as {selector}IsEnabled, so if you implement -paste: and -pasteIsEnabled then you can turn off paste by returning NO to -pasteIsEnabled.

David


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Josh Freeman
In reply to this post by David Chisnall
Hi Riccardo,

On Nov 13, 2017, at 5:24 AM, David Chisnall wrote:

> On 13 Nov 2017, at 08:17, Wolfgang Lux <[hidden email]> wrote:
>>
>> While sel_isEqual is just an alias for == on Apple's runtime (and  
>> the GNUstep runtime, I assume), this need not be the case for other  
>> runtimes. In particular, for the old GNU runtime sel_isEqual is not  
>> equivalent to ==, as you've observed.
>
> It’s also not == on the GNUstep runtime, and can’t easily be with a  
> new ABI if we want it to work on ELF / COFF platforms.  Apple relies  
> on the run-time linker deduplicating their selector table and  
> inserting selectors into a contiguous region of memory.


    Note that Apple didn't add sel_IsEqual() to its runtime until OS X  
10.5; If you still want Graphos to support OS X 10.4 & earlier, you'll  
need to keep using '==' on Macs. One way to do this without #ifdefs  
around each selector comparison would be to use a macro:


#ifdef __APPLE__
#   define macroSelectorsAreEqual(selector1, selector2) (selector1 ==  
selector2)
#else // GNUstep
#   define macroSelectorsAreEqual(selector1, selector2)  
sel_isEqual(selector1, selector2)
#endif

...

if (macroSelectorsAreEqual(action, @selector(copy:))) {...


    (It's probably safe to assume Apple's runtime will continue  
supporting '==' for selector comparison, otherwise it would break a  
significant amount of legacy code).

Cheers,

Josh


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

David Chisnall
On 13 Nov 2017, at 18:14, Josh Freeman <[hidden email]> wrote:
>
>   (It's probably safe to assume Apple's runtime will continue supporting '==' for selector comparison, otherwise it would break a significant amount of legacy code).

No it isn’t.  Apple has no problems breaking legacy code and this has been a warning on Apple platforms with a warning that it may break in the future for a decade.

There is no reason to make code ugly with macros to support a version of OS X that has been out of support for years and at this point has so many known and unpatched security vulnerabilities that using it in any situation where it may come into contact with untrusted data would be a terrible idea.

David


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Wolfgang Lux
In reply to this post by David Chisnall
> Am 13.11.2017 um 19:13 schrieb David Chisnall <[hidden email]>:
>
>> On 13 Nov 2017, at 17:52, Wolfgang Lux <[hidden email]> wrote:
>>
>>> Would it make sense to not have custom code to enable/disable menu items, and just have appkit do it for you depending on the target?
>>
>> In my experience, custom code is unavoidable in almost any non-trivial case because the AppKit is not able to determine exactly when a menu item is applicable and when not. Having support for the selectors is a necessary but not a sufficient condition.
>
> IOKit improves this relative to AppKit by adding the delegates to the responder chain.  For custom views, I’ve taken to doing this and forwarding some of the responder chain queries to the delegate.  It would also be nice if there were some convention such as {selector}IsEnabled, so if you implement -paste: and -pasteIsEnabled then you can turn off paste by returning NO to -pasteIsEnabled.

Well, that almost exists in the form of -validateMenuItem: and -validateUserInterfaceItem:. It’s just that you implement just a single method that receives a selector argument rather than an individual method for each selector.

Wolfgang
_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

David Chisnall
On 13 Nov 2017, at 19:58, Wolfgang Lux <[hidden email]> wrote:

>
>> Am 13.11.2017 um 19:13 schrieb David Chisnall <[hidden email]>:
>>
>>> On 13 Nov 2017, at 17:52, Wolfgang Lux <[hidden email]> wrote:
>>>
>>>> Would it make sense to not have custom code to enable/disable menu items, and just have appkit do it for you depending on the target?
>>>
>>> In my experience, custom code is unavoidable in almost any non-trivial case because the AppKit is not able to determine exactly when a menu item is applicable and when not. Having support for the selectors is a necessary but not a sufficient condition.
>>
>> IOKit improves this relative to AppKit by adding the delegates to the responder chain.  For custom views, I’ve taken to doing this and forwarding some of the responder chain queries to the delegate.  It would also be nice if there were some convention such as {selector}IsEnabled, so if you implement -paste: and -pasteIsEnabled then you can turn off paste by returning NO to -pasteIsEnabled.
>
> Well, that almost exists in the form of -validateMenuItem: and -validateUserInterfaceItem:. It’s just that you implement just a single method that receives a selector argument rather than an individual method for each selector.

Yup, and whenever I write one of these I end up with a horrible blob of if statements.  In a language with dynamic dispatch, it would be nice for the menu item to just do NSSelectorFromString once, get the method that dynamically [de]activates it, and call that.  We’re already doing a bunch of message sends to deliver things down the responder chain, one more wouldn’t have visible overhead.

David


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Wolfgang Lux


> Am 14.11.2017 um 11:01 schrieb David Chisnall <[hidden email]>:
>
> On 13 Nov 2017, at 19:58, Wolfgang Lux <[hidden email]> wrote:
>>
>>> Am 13.11.2017 um 19:13 schrieb David Chisnall <[hidden email]>:
>>>
>>>> On 13 Nov 2017, at 17:52, Wolfgang Lux <[hidden email]> wrote:
>>>>
>>>>> Would it make sense to not have custom code to enable/disable menu items, and just have appkit do it for you depending on the target?
>>>>
>>>> In my experience, custom code is unavoidable in almost any non-trivial case because the AppKit is not able to determine exactly when a menu item is applicable and when not. Having support for the selectors is a necessary but not a sufficient condition.
>>>
>>> IOKit improves this relative to AppKit by adding the delegates to the responder chain. For custom views, I’ve taken to doing this and forwarding some of the responder chain queries to the delegate.  It would also be nice if there were some convention such as {selector}IsEnabled, so if you implement -paste: and -pasteIsEnabled then you can turn off paste by returning NO to -pasteIsEnabled.
>>
>> Well, that almost exists in the form of -validateMenuItem: and -validateUserInterfaceItem:. It’s just that you implement just a single method that receives a selector argument rather than an individual method for each selector.
>
> Yup, and whenever I write one of these I end up with a horrible blob of if statements.  In a language with dynamic dispatch, it would be nice for the menu item to just do NSSelectorFromString once, get the method that dynamically [de]activates it, and call that. We’re already doing a bunch of message sends to deliver things down the responder chain, one more wouldn’t have visible overhead.

So next time you probably should define your validateMenuItem: like this (beware, untested code):

- (BOOL) validateMenuItem: (SEL)aSelector
{
  aSelector = NSSelectorFromString([NSStringFromSelector(aSelector) stringByAppendingString: @"IsEnabled"]);
  if ([self respondsToSelector: aSelector])
    return (BOOL)[self performSelector: aSelector];
  return YES;
}

And then you can implement -pasteIsEnabled, -copyIsEnabled, etc. as you like. :-)

Wolfgang


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Riccardo Mottola-5
In reply to this post by Josh Freeman
Hi Josh,

On 2017-11-13 19:14:34 +0100 Josh Freeman
<[hidden email]> wrote:

>
>
>     Note that Apple didn't add sel_IsEqual() to its runtime until OS
> X  10.5;
> If you still want Graphos to support OS X 10.4 & earlier, you'll  
> need to
> keep using '==' on Macs. One way to do this without #ifdefs  around
> each
> selector comparison would be to use a macro:
>

Of course I want to support them.. so nice to have stuff run on my
iBook and PowerBook !

>
> #ifdef __APPLE__
> #   define macroSelectorsAreEqual(selector1, selector2) (selector1 ==
> selector2)
> #else // GNUstep
> #   define macroSelectorsAreEqual(selector1, selector2)
> sel_isEqual(selector1, selector2)
> #endif
>

I think this is finer:

#if !defined (GNUSTEP) &&  (MAC_OS_X_VERSION_MAX_ALLOWED <=
MAC_OS_X_VERSION_10_4)
#define sel_isEqual(selector1, selector2) (selector1 ==  selector2)
#endif

So it is just there for "old" MacOS and GNUstep and more modern MacOS
are fine.
I wonder if the limit is 10.4 or 10.5, on 10.6 sel_isEqual works.

Thanks and have fun playing with Graphos :)

Riccardo


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

Riccardo Mottola-5
In reply to this post by David Chisnall


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep
Reply | Threaded
Open this post in threaded view
|

Re: issue with auto-enable of Menus

David Chisnall
In reply to this post by Wolfgang Lux
On 14 Nov 2017, at 21:19, Wolfgang Lux <[hidden email]> wrote:

>
> So next time you probably should define your validateMenuItem: like this (beware, untested code):
>
> - (BOOL) validateMenuItem: (SEL)aSelector
> {
>  aSelector = NSSelectorFromString([NSStringFromSelector(aSelector) stringByAppendingString: @"IsEnabled"]);
>  if ([self respondsToSelector: aSelector])
>    return (BOOL)[self performSelector: aSelector];
>  return YES;
> }

That’s not quite right, because you need to strip the trailing : from `NSStringFromSelector(aSelector)`.  It’s a bit annoying to do it here, because the NSMenuItem could cache the selector once per program invocation, rather than having to reconstruct it with string manipulation on each call.  Probably not a performance difference that’s noticeable to a human, but it seems inelegant.

Unfortunately, that’s what we’re stuck with if we want OpenStep / Cocoa compat.

David


_______________________________________________
Discuss-gnustep mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/discuss-gnustep