Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reshuffle of services and add trigger for ⌘⎋. Fixes #409 #1295

Merged
merged 2 commits into from
Jan 29, 2013

Conversation

pjrobertson
Copy link
Member

This commit is actually quite simple when it comes down to it. I've made the existing 'Select Current Selection in Command Window' trigger be enabled by default and use the ⌘⎋ shortcut.
I've also removed the 'Send to Quicksilver' service, meaning we now only have one.

Notes:

  • Some users may have to re-configure the trigger if they've been using the 'Send to Quicksilver' service (now that it's deleted). But services rarely get flushed/reconfigures so there may well be no need.
  • I decided not to merge the Finder and Global Selection code. I think we all understand a bit better now how the two bits of code differ, and when to use one or the other. We also know the limitation of getting stuff from the Finder, so I don't see it work us merging the two.
  • The service and the trigger have the same keyboard shortcut, this seems to work fine - we just need to advertise that only keyboard shortcut the trigger be changed
  • This is quite a useful read to figure out a bit more about debugging services
  • For debugging purposes, using a Guest account helps, but I could only run release builds on the guest account
  • To reset your Services, make sure you delete all copies of QS (including those in the trash) and run /System/Library/CoreServices/pbs -flush. Check the 'Keyboard Shortcuts' sys prefs following this to see the state of services
  • We could setup the service to use an even more obscure shortcut (like ⌘⌥⌃⎋), so it won't ever realistically conflict with anything else, but I don't see the point

@tiennou
Copy link
Member

tiennou commented Dec 17, 2012

Why do you say "removed the Send to Quicksilver" service ? The ultimate advantage the service has above the trigger is that the trigger works even with QS not running, so I don't like that sentence ;-).

@pjrobertson
Copy link
Member Author

Good point.

The 'Get Current Selection' service still exists, so that can be used to launch QS if it's not running. I really don't think there's a need to have two services though (one that the user can change the keyboard shortcut for, and one that's 'fixed')

  • How often will QS not be running and you decide 'oh, I'll just grab this selection'.
  • Currently, the service and the selection are set up to use ⌘⎋, so for 99.9% of users who have always been using ⌘⎋ (like we told them to), then nothing will change
  • I've found that having multiple copies of QS lying around, and using ⌘⎋ even when one instance of QS is running will launch another from a different .app. Strange!

@tiennou
Copy link
Member

tiennou commented Dec 17, 2012

I don't remember clearly that part of the code ;-). Is there a specific reason for the shortcut being fixed ? What would happen if I change it through Keyboard preferences ?

@pjrobertson
Copy link
Member Author

I don't remember clearly that part of the code ;-). Is there a specific
reason for the shortcut being fixed ? What would happen if I change it
through Keyboard preferences ?

The keyboard shortcut is hard-corded in QSGlobalSelectionProvider
(invokeService:). So it you change the shortcut the 'current selection'
proxy will break.
I had considered changing this hard-coded keyboard 'press' depending on the
service settings, but I thought it best to just allow users to use a
trigger instead. It's kind of more obvious, more customisable, and allows
for more shortcuts (Services shortcuts HAVE to have ⌘ in them).

On 17 December 2012 08:20, Etienne Samson notifications@github.com wrote:

I don't remember clearly that part of the code ;-). Is there a specific
reason for the shortcut being fixed ? What would happen if I change it
through Keyboard preferences ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1295#issuecomment-11433457.

@skurfer
Copy link
Member

skurfer commented Dec 17, 2012

My only objection (which is not a very strong one) is that this sort of implies that the trigger and the service are two ways to do the same thing, and the trigger exists so you can customize the shortcut. But they're not the same thing.

I'm sure this is documented, but discussion is all over the place and I've forgotten most of it, so just to recap differences I've found:

  • The service always seems to get the correct selection from Finder. The trigger suffers from the known bug.
  • The service seems to have the result ready in the first pane much faster.
  • The service works with selected text in OmniWeb. The trigger doesn't.
  • The trigger works in cases where the "column view" bug prevents services from being listed.
  • The trigger works to get the current folder if no files are selected. The service isn't listed with no selection.

So in other words, the service is universally more reliable. The only situations where the trigger works better are those where the service isn't available at all. I don't suppose there's a way to make the trigger (or another trigger) do what the service does? Seems like that depends on an NSPasteboard object from the OS or something, so probably not.

@pjrobertson
Copy link
Member Author

True, they're not really the same thing (nice list of differences btw). But isn't the whole point to try and reduce the number of different things so things are less confusing?!
I don't think the general user really needs to know that there are even two different things (a service and an applescript for getting the selection), just that they can grab the current selection with whatever command they set up.

Going through your points, it seems like we can't really win (use the trigger for certain cases where it works and the service for other cases). If people really know the difference, or want the 'old way' back, they can just turn off the trigger/change its hotkey.

The service seems to have the result ready in the first pane much faster.

OK, see the latest commit which seems to speed this up so runs almost as quick as the service (maybe I'm getting old, but 0.1s seems pretty quick ;-) )

The trigger works in cases where the "column view" bug prevents services from being listed.

Just realised now, but this is probably a biggie for me. To not have to think about this bug would be great. I normally try the service, remember it doesn't work, switch to thumbnail view and then use ⌘⎋ again. Horrible.

I don't suppose there's a way to make the trigger (or another trigger) do what the service does?

We could create another trigger that just basically calls the service method (calls invokeService), but do we really want to make another trigger for getting the current selection?


The only other downside I can see is that the trigger caches the result for 3s, since we're using a proxy. Is it that bad? If it is, turn off the trigger :)

@skurfer
Copy link
Member

skurfer commented Dec 19, 2012

We could create another trigger that just basically calls the service method (calls invokeService), but do we really want to make another trigger for getting the current selection?

I'd use the shit out of it starting today! I mean… yeah, I think so. :-) Or, if you don't like the trigger idea, would it be possible to get the result and stick it in a new proxy object? Then people could create their own trigger for "Whatever → Select in Command Window". Or we could go crazy and just make that the "global selection" proxy, and just like that, all these different methods use the same code. (I'm probably overlooking something.)

@pjrobertson
Copy link
Member Author

I think I never replied to your comment @skuerfer because I didn't really understand what you meant :)

Or we could go crazy and just make that the "global selection" proxy, and just like that, all these different methods use the same code. (I'm probably overlooking something

You mean, a proxy object that invokes the service and call it 'global selection'? That's exactly what the current selection proxy does, and in this pull I've made it into a trigger on ⌘⎋

Just trying to get this wrapped up so we can get v1.0 out :D

@skurfer
Copy link
Member

skurfer commented Jan 25, 2013

Yeah, I realized that the other day when I saw weird stuff in TextMate when there was no selection. It was because ⌘⎋ does some sort of completion. So maybe we should use something more obscure after all.

So going through my list again, this seems to fix everything but the incorrect Finder selection bug. Wonder why. If I disable the trigger and use the service directly, it's fine.

I withdraw my wishy-washy objection… I think. :-)

@pjrobertson
Copy link
Member Author

I explained on IRC that I couldn't fix the Finder selection bug, as this would mean activating another app then going back to Finder - which causes all sorts of unwanted things. Well now we have a discussion for that at #1351, so I won't talk about it here :)

@skurfer
Copy link
Member

skurfer commented Jan 29, 2013

How am I going to summarize this in the release notes? :-)

skurfer added a commit that referenced this pull request Jan 29, 2013
Reshuffle of services and add trigger for ⌘⎋. Fixes #409
@skurfer skurfer merged commit 8cae4eb into quicksilver:master Jan 29, 2013
@pjrobertson pjrobertson deleted the services branch February 1, 2013 21:04
@daniels220
Copy link

I use Cmd-Esc to summon the iTerm2 Visor. I can't change the shortcut in the QS triggers prefs (I'd prefer Ctrl-Opt-Space). When I restart QS it just goes back to being Cmd-Esc, and it does in fact try to activate (which messes up iTerm2). Can I have a way to change that shortcut, please?

@pjrobertson
Copy link
Member Author

This is because the underlying services shortcut is still set to ⌘⎋ and is
still being used even after you change the trigger. We have discussed using
a more 'obscure' keyboard shortcut for the service - @skurfer should we
implement this for ß72?

On 16 February 2013 04:46, daniels220 notifications@github.com wrote:

I use Cmd-Esc to summon the iTerm2 Visor. I can't change the shortcut in
the QS triggers prefs (I'd prefer Ctrl-Opt-Space). When I restart QS it
just goes back to being Cmd-Esc, and it does in fact try to activate (which
messes up iTerm2). Can I have a way to change that shortcut, please?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1295#issuecomment-13644489.

@skurfer
Copy link
Member

skurfer commented Feb 16, 2013

Is the problem that the trigger can't be changed, or that it calls the service by “pressing” ⌘⎋? It sounds like the former.

So if you go to Triggers → Quicksilver → Command Window with Selection and change the keystroke, it doesn't stick after a restart?

@daniels220
Copy link

Yes, the problem is that the trigger can't be changed, i.e. doesn't stick after a restart. It can be changed temporarily, changing it in the prefs window does change what shortcut QS responds to, but it doesn't stick. (And @pjrobertson Visor catches the shortcut before it triggers the underlying service, so I don't think I actually need the underlying service shortcut changed—although I think it makes sense to do so, since it's a shortcut the user should never use.)

@pjrobertson
Copy link
Member Author

Strange. Are you restarting QS immediately after changing the trigger
shortuct? Quicksilver takes ~5s to register the trigger change, so try
closing the prefs window, changing the shortcut, then waiting a few seconds
before restarting.

On 16 February 2013 16:55, daniels220 notifications@github.com wrote:

Yes, the problem is that the trigger can't be changed, i.e. doesn't stick
after a restart. It can be changed temporarily, changing it in the prefs
window does change what shortcut QS responds to, but it doesn't stick. (And
@pjrobertson https://github.com/pjrobertson Visor catches the shortcut
before it triggers the underlying service, so I don't think I actually need
the underlying service shortcut changed—although I think it makes sense to
do so, since it's a shortcut the user should never use.)


Reply to this email directly or view it on GitHubhttps://github.com//pull/1295#issuecomment-13658325.

@daniels220
Copy link

Yes, I waited a full minute to restart QS and the change doesn't stick. Actually the way I discovered the problem was I changed the shortcut and the next day when I rebooted the machine it was back to Cmd-Esc.

@skurfer
Copy link
Member

skurfer commented Feb 18, 2013

since it's a shortcut the user should never use.

I wouldn't say that. The reason it's ⌘⎋ by default is people have used that to invoke the service directly for years. Some people might want to continue doing that, depending on which problems they find more annoying.

@daniels220
Copy link

True, the situation with the Finder really sucks... although if I'm to use the shortcut for the service, I'd like to be able to change it. Honestly I haven't been following this discussion that closely, but it seems like the way it was before with two different services was better--that way one could be changed and the other would be used internally.

In any case, any idea what the problem is with being unable to change the trigger shortcut?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants