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

Hide obsolete details #613

Merged
merged 3 commits into from
Jan 12, 2012
Merged

Conversation

pjrobertson
Copy link
Member

Can't believe we've never really thought about this before.

Quicksilver displays an object's name and details regardless of whether the two are the same.
This clutters up the interface with duplicate text.

Typically, it happens with URLs and text. The place where I saw it today and thought 'huh?!' was in the clipboard window. It looks so messy!

This small change only displays the details if they're different from the name. There are a few small discrepancies: sometimes details strings are clipped, so the start is the same as the name, but not the whole string (and my changes fail).
Also, it seems like the name string has the whitespace stripped out of it, whereas the details string doesn't.

Yay a tidy up! :)

@pjrobertson
Copy link
Member Author

P.S. the reverted commit has been pushed to master, so ignore that

@skurfer
Copy link
Member

skurfer commented Dec 17, 2011

In attempting to fix #591, I may have stumbled upon a better fix for this.

Related note: Holy shit! Did you know actions had details? Helpful details. Everything in QSAction.description.strings has been getting ignored.

Pull request coming, but I’m still trying to figure it all out.

@skurfer skurfer mentioned this pull request Dec 19, 2011
@skurfer
Copy link
Member

skurfer commented Jan 11, 2012

Should we close this? The fix in #591 seems much simpler. :-)

@pjrobertson
Copy link
Member Author

Does #591 / #624 fix displaying the same thing twice? E.g. if you look at the clipboard (if there's text in it) you get duplicate text.

If it does, then I guess the localisable fix is the proper/correct way of doing things :-)

-- see my comments on your #624 pull request

@skurfer
Copy link
Member

skurfer commented Jan 12, 2012

Does #591 / #624 fix displaying the same thing twice?

Interesting. With my change merged in, duplicates were gone for most things, but text in the clipboard history was duplicated (when viewed as children of the clipboard). Merged in your change and now that’s fixed too.

I see now that I’m only checking for a match with the details stored in the object. There are other potential sources of details in that method.

  1. I would have to add several string comparisons all over that method to catch all the possibilities.
  2. Other parts of the code (or plug-ins) might have legitimate reasons to request the details from the object. It shouldn’t lie and say there are none.
  3. Related to 2, whether or not the details should be displayed should be decided at the display level (where you implemented it). Not lower down.

For those reasons, I will take out my changes to the details method. The other changes in that pull request should be good though. You shouldn’t change anything. I’ll make my changes and test along-side this. If all looks good, I’ll merge this one.

skurfer added a commit that referenced this pull request Jan 12, 2012
@skurfer skurfer merged commit acb618c into quicksilver:master Jan 12, 2012
@pjrobertson
Copy link
Member Author

Cool, so this is all good? :-)

See my comment over on the 'other' pull request!

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.

2 participants