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

Show URLs have children, don't show text has children #859

Merged
merged 8 commits into from May 3, 2012

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Apr 30, 2012

Previously, text was set to return YES for objectHasChildren meaning there was always a small '>' arrow next to text objects.

In the same manner, URLs were also set to return NO for objectHasChildren so the '>' would never show for URLs.

Regarding the URLs - I have added the objectHasChildren method, which checks if the URL has a corresponding QSURLParser available for it.
If it does, then it saves the type of URL parser in the object's meta for easy collection later on. Otherwise, it would be required to iterate through all the .tlds again to find the type.

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 30, 2012

Works fine, but I don’t like the QSURLParserType business. It’s just a string used as a key in the metadata, but it’s being set up to look like a type, which in all other cases is used with setObject:forType:. So I would either use setObject:forType: instead of forMeta: here or don’t use “Type” in the name of the key.

Mainly trying to prevent confusion for someone who looks at this in the future.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 30, 2012

OK I've tried to make it clearer now, and have added some comments :)

On 30 April 2012 21:01, Rob McBroom <
reply@reply.github.com

wrote:

Works fine, but I don’t like the QSURLParserType business. It’s just a
string used as a key in the metadata, but it’s being set up to look like a
type, which in all other cases is used with setObject:forType:. So I
would either use setObject:forType: instead of forMeta: here or don’t
use “Type” in the name of the key.

Mainly trying to prevent confusion for someone who looks at this in the
future.


Reply to this email directly or view it on GitHub:
#859 (comment)

@tiennou
Copy link
Member

@tiennou tiennou commented May 1, 2012

I can't remember where I understood that, but I think the arrow next to text objects was there to allow access to lines inside some files or something like that... Yes, it looks like that, there's a bunch of QSParser subclasses that fit in the picture, for links in HTML files, items in plist data, and lines in text files.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 1, 2012

I can't remember where I understood that, but I think the arrow next to
text objects was there to allow access to lines inside some files or
something like that... Yes, it looks like that, there's a bunch of QSParser
subclasses that fit in the picture, for links in HTML files, items in plist
data, and lines in text files.

We discussed this on IRC yesterday - you're right, you can right arrow into
.txt files etc. But never into text objects. This change only removes the >
from text objects, not text files.

Having said that, .txt files don't have the the '>' next to them anyway!
Something else to fix here... :)

I've added another commit that checks whether there are valid parsers for
file objects, if so then show the '>'.
Should work for .html and .txt files now

I have also tried to prioritise the order to check for children, in my
opinion (total guess) the following list (from highest to lowest) shows how
often I have a certain file type displayed in QS:

Folder
App
.txt or .html file
Alias

meaning that the checks to check if the object has children is done in this
order. The only (possible) change this'll have is that I've moved the check
on folders to the top - the most likely by far I think

On 1 May 2012 12:55, Etienne Samson <
reply@reply.github.com

wrote:

I can't remember where I understood that, but I think the arrow next to
text objects was there to allow access to lines inside some files or
something like that... Yes, it looks like that, there's a bunch of QSParser
subclasses that fit in the picture, for links in HTML files, items in plist
data, and lines in text files.


Reply to this email directly or view it on GitHub:
#859 (comment)

…the file has children

Also - the 'QSFSFileTypeChildHandlers' table doesn't seem to exist anywhere, so I've removed this bit of code
@skurfer
Copy link
Member

@skurfer skurfer commented May 2, 2012

I like the smarts based on handlers and parsers. For some reason, I’m not seeing the arrow next to applications with recent documents. Any ideas?

skurfer added a commit that referenced this issue May 3, 2012
Show URLs have children, don't show text has children
@skurfer skurfer merged commit 0d55743 into quicksilver:master May 3, 2012
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

3 participants