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

Synonyms #1325

Merged
merged 24 commits into from Jan 31, 2013
Merged

Synonyms #1325

merged 24 commits into from Jan 31, 2013

Conversation

skurfer
Copy link
Member

@skurfer skurfer commented Jan 16, 2013

Here's an official implementation of a long sought-after feature. This allows users to give something an additional name. For example, you could:

  • refer to Colloquy as "IRC Client"
  • refer to TextEdit as "Notepad" (I've had someone ask about that)
  • refer to Contacts as "Address Book" (if you just can't train yourself to search for the new name)

Under the hood, these are proxy objects. While long-time Quicksilver users are familiar with that term, I chose to obscure the implementation details here because:

  1. They've never been able to define their own, so this is all new.
  2. We should have a name that makes the purpose obvious to new and old users.
  3. "User Defined Proxy Object" doesn't exactly roll off the tongue, or the fingers.

Note that I've altered some of the typical proxy object behavior to make synonyms act more like the thing they refer to. (Right-arrowing, for example.)

There might be some visual tweaks we want to make here and there, but as far as functionality goes, I don't know of any problems.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 17, 2013

Nice, all squished into one commit :)

I've got my notes out, and you've fixed two out of the 3 bugs ;-)

Here's the last straggler I had written down. I'll look for more as well :)

  • BUG TWO
  • create a new synonym and select an object (by clicking 'click to change', finding an object then pressing )
  • 'Click to change' and select another object, but hit ⎋ to dismiss the window (do NOT press )
  • Edit the name of the object and press .
  • You'll notice the synonym object changes to the dismissed object

A few more things:

  • should the name in the catalog prefs be "Synonym name" > "Actual name"? because it doesn't work for me, it's just kinda weird (datafeeds.networkrail.co.uk is a 1Password login object I have in my catalog)
  • I can create a synonym to the synonym itself (by setting up a synonym, then changing the object to itself). After a relaunch this falls apart. Maybe filter synonym proxies out of the 'click to change' window?
  • Similar to the above, if I create a synonym to an object not in my catalog a restart makes things fall apart (the usual saving objects problem)
  • If I set up a file to be a synonym, ⌥→ doesn't work
  • Can enter text mode and do all sorts of funky things, but I can't save it (which makes sense, perhaps text mode should just be disabled)
  • Finding a synonym, pressing → then ← then →, I can't go back afterwards. But I think this is a bigger problem. The exact same thing happens if I do it for a Contact obj
    �- [ ] Right clicking on the object input pane brings up an 'actions' menu, but I kinda like this as it just opens the actual QS window :)
  • Drag and drop works - nice, but if I drag a file from the add object window, then put it back, it disappears. Drag needs to be disabled, leaving drop enabled I think.

Sorry for the long list, lots of them are just me being pedantic and OTT ;-)

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 17, 2013

P.S. that bug seems kind of similar to @tiennou's fix on #1227.
Perhaps it's a bigger problem than just in your changes

P.P.S. I like the name synonyms, and agree RE obscuring 'proxy objects' from the user in this case

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 17, 2013

Nice, all squished into one commit :)

Yeah, I got everything right on the first try, of course. ;-)

I've got my notes out, and you've fixed two out of the 3 bugs ;-)

Nothing has changed from the other branch I shared. I suspect these problems went away when master and release were merged (which is why I was waiting for that to happen before submitting this).

create a new synonym and select an object (by clicking 'click to change', finding an object then pressing )
'Click to change' and select another object, but hit ⎋ to dismiss the window (do NOT press )
Edit the name of the object and press .
You'll notice the synonym object changes to the dismissed object

Yeah, it refers to representedObject no matter what. I'll need to fix that. Good catch.

should the name in the catalog prefs be "Synonym name" > "Actual name"? because it doesn't work for me, it's just kinda weird (datafeeds.networkrail.co.uk is a 1Password login object I have in my catalog)

I'd prefer to make it → instead of >. Forgot to go change that.

It usually takes a relaunch (or a long wait) before the UI shows the right name. I was hoping one of your catalog UI refreshing changes would make it appear correctly right away, but it clearly hasn't. Not sure what the right thing to do is here. Leave the name alone and let users set it?

I can create a synonym to the synonym itself (by setting up a synonym, then changing the object to itself). After a relaunch this falls apart. Maybe filter synonym proxies out of the 'click to change' window?

Well, don't do that. :-) I'm not sure how we could prevent it. There's no real way to tell a Synonym apart from any other proxy. We could add some metadata to each one I guess, but is it worth it?

Similar to the above, if I create a synonym to an object not in my catalog a restart makes things fall apart (the usual saving objects problem)

Known problem. There are probably some messages in your console about it. I was thinking of making a change here that would allow it to eventually work, but I don't know what our eventual approach (to resolve #1277) is going to be. Actually, looking at that issue, I actually said "This has been a problem with triggers for a long time, and will soon be a problem for Synonyms." :-)

Anyway, that's a larger issue. I'll comment over there.

If I set up a file to be a synonym, ⌥→ doesn't work

True. I'll see if there's a way.

Can enter text mode and do all sorts of funky things, but I can't save it (which makes sense, perhaps text mode should just be disabled)

I thought I had. Must have gotten removed during some trial and error …except that I did it all in one commit, remember?

Right clicking on the object input pane brings up an 'actions' menu, but I kinda like this as it just opens the actual QS window :)

Can we prevent that? Should we?

Drag and drop works - nice, but if I drag a file from the add object window, then put it back, it disappears. Drag needs to be disabled, leaving drop enabled I think.

Makes sense. No idea how to do it. :-)

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 18, 2013

The new commits should fix everything but dragging out of the target picker.

And I still can't get it to center over the button. [settingsView convertRect:[targetPickerButton frame] toView:nil] doesn't seem to convert it to anything. It still appears in the lower-left corner of the screen with the same offset the targetPickerButton has relative to the settingsView.

The red and green lines shown here are the same, in other words.

Screen Position

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 20, 2013

I've done the following:

  • Given the target picker its own QSTargetPickerPanel class
  • Moved init code to this class, so it's not called every time the window's open
  • Made the window zoom from the button as you wanted - sorry I forgot to mention you need to convert to window co-ords then to screen
  • Fixed the drag/drop bug

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 20, 2013

another commit to fix selecting the right object.

Note to self:
QSSearchObjectView has 3 methods for 'selecting' an object:

- (void)setSelectedObject:(id)newSelectedObject;
- (void)selectObjectValue:(QSObject *)newObject;
- (void)selectObject:(QSBasicObject *)obj;

which one's the right one? Who knows ;-)

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 20, 2013

Oh, and you probably want to see my commits. Sorry they're not as tidy as your 'one commit' ;-)

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 20, 2013

...and finally one more commit to correctly update the listing in the Catalog prefs :)

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 21, 2013

Looks good. Thanks!

So we need two classes dedicated exclusively to that stupid target picker window? Annoying.

Just a couple of small things I want to change and I'll push some more commits.

  1. The window starts over the object in the panel, but it still ends up in the center of the screen.
  2. You've removed the checks that allowed users to rename the catalog entry. I see now that those checks don't work any more, so I'll try to update them.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 21, 2013

The window starts over the object in the panel, but it still ends up in the center of the screen.

Did you not want it to zoom to the middle? Oh, I misunderstood. Just fiddle with my code in -(IBAction)showTargetPicker:(id)sender then. The centerRect is pretty well documented.

You've removed the checks that allowed users to rename the catalog entry

Aah so is that why you were updating the name of the item only if it hadn't previously been set. It meant it was just never updating. What's the point of editing the catalog entry name anyway? What benefits does it have? I'd say that seeing the correct name (less confusing) is far more important than allowing people to update the name

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 21, 2013

Did you not want it to zoom to the middle?

I wanted it to appear directly above the target in the info panel. I've got that working now thanks to your enormous head start. I would also like to match the color of the info panel, but the standard-issue light gray doesn't appear to be one of NSColor's presets, so I went with the colors from the prefs.

What's the point of editing the catalog entry name anyway? What benefits does it have? I'd say that seeing the correct name (less confusing) is far more important than allowing people to update the name

Well, mostly just because you've always been able to do it in the past. But given the difficulty of detecting a user defined name vs. an older preset, I think I'm going to agree with you and just force a nice name. :-)

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 22, 2013

Just rebased against the latest master, so pull down a fresh copy if you want to test. Or just merge it if you were fine with the way it was before. All I did was remove your one commit that was over in optimisations.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 22, 2013

Added that method to QSInterfaceController.h.

I also noticed that text entry mode isn't being disabled since you moved the search object view stuff to the new class. Any ideas? Clean build fixes this

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 23, 2013

All looks good. I've tweaked the target picker window a little further so it blends in better with the sidepanel, and keeps it's size relative to the sidebar size better.
I think it's how you mentioned you wanted it, but I'm not sure

Feel free to cherry-pick or ignore.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 23, 2013

OK, more commits.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 23, 2013

Almost there, just noticed a funny thing:

  • open the target picker window
  • invoke the actual QS window
  • click somewhere in the sidebar (not in the target picker window)
  • Note how the target picker window disappears to behind the prefs window (move the prefs to see it)

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 23, 2013

To fix that ^, just add this to QSTargetPickerPanel.m

-(void)resignMainWindow {
    [self orderOut:nil];
}

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 23, 2013

Fixed. Oh, and just use both sides to resolve the conflict.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 25, 2013

Oh yeah, there's one last thing (as always!).
The QSUserDefinedProxySource.xib is still set to deployment: 10.8.

Once that's done, merge away into maser as you see fit! :)

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 25, 2013

I thought of one more thing too. I'd like to have some idea where we're going with #1277 before finalizing this. In other words, I want to decide on a format for storing the target so, long term, we can create synonyms for things outside the catalog.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 29, 2013

OK, rewrote history here to change the last commit from handler to type. And since I was rewriting history anyway, I rebased this against master to get rid of the merge conflict. It should be ready.

skurfer added a commit that referenced this issue Jan 31, 2013
@skurfer skurfer merged commit 9a1e822 into quicksilver:master Jan 31, 2013
@skurfer skurfer deleted the synonyms branch Jan 31, 2013
@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 31, 2013

Not sure if you saw, but I fixed the deployment for the NIB. I want to have this merged in so I can include it in the proof-of-concept for recreating objects.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 31, 2013

Fine by me.

I only left it unmerged because I wasn't sure if your change was enough for
recreating objects.
But I guess if you find down the line it isn't then you can just change it

  • fine by me

On 31 January 2013 13:46, Rob McBroom notifications@github.com wrote:

Not sure if you saw, but I fixed the deployment for the NIB. I want to
have this merged in so I can include it in the proof-of-concept for
recreating objects.


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

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jan 31, 2013

I only left it unmerged because I wasn't sure if your change was enough for recreating objects.

Yes and no. It was part of it, but other changes were needed (just to make sure the type was propagated from the plist down to the place where it's needed). You'll see.

@philostein
Copy link
Contributor

@philostein philostein commented Feb 10, 2013

This works great! Like the way the object field in Catalog Prefs acts like QS pane 1.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Feb 10, 2013

This works great! Like the way the object field in Catalog Prefs acts
like QS pane 1.

It basically is the QS pane 1!
Some trickery and clever blending makes it look fancy but ssssh don't tell
anyone! ;-)

Indeed Rob's done an awesome job :)

On 10 February 2013 11:34, philostein notifications@github.com wrote:

This works great! Like the way the object field in Catalog Prefs acts like
QS pane 1.


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

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 10, 2013

I had help, but thanks. :-)

@philostein
Copy link
Contributor

@philostein philostein commented Feb 27, 2013

Expansion idea: For folders, have the synonym work in pane 3 also. Dunno if it's possible seeing as they're really proxies.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 27, 2013

I knew that would be a limitation, but I think we need to fix the more general problem of allowing proxies in the third pane. (That will automatically take care of synonyms.)

@Joshfindit
Copy link

@Joshfindit Joshfindit commented Mar 26, 2013

This is slightly off-topic, but thank you for this: the ability to 'rename' items is the single function that has been missing from my OSX experience.

I even installed Alfred 2, thinking that this single feature would be the reason I switched. Happy to be proven wrong!

@skurfer
Copy link
Member Author

@skurfer skurfer commented Mar 27, 2013

It's something new users have asked about since I was a new user. I'm glad people find it useful.

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