quicksilver / Quicksilver

Quicksilver Project Source
http://qsapp.com
Apache License 2.0
2.72k stars 283 forks source link

Give file objects a string value #2887

Closed pjrobertson closed 2 months ago

pjrobertson commented 2 years ago

Useful for when a string type is required e.g. when combining multiple objects (string + file)

Fixes #1673


I'm opening a PR for this since I note that originally @skurfer had earmarked it to fix. The fix I implemented is essentially what I said in this comment.

I'm not sure of the consequences of giving file objects a 'string type' from the start, so I think it's worth @skurfer running with this build for a while.

P.S If you can download a debug version from here without having to build yourself ;-) https://github.com/quicksilver/Quicksilver/actions/runs/2483657221

skurfer commented 2 years ago

The impact of this probably varies from user to user. Basically, anyone that has an action that applies to text higher up on the list than Open (or whatever they want as the default for files) could get an unpleasant surprise. Easily fixed, but not sure if people would know that. It’s probably worth it to make this change, though.

pjrobertson commented 2 years ago

So I've been thinking about this more recently, and this would fix the issue for File Objects, but not other objects types that don't have a string type still wouldn't work (in the issue above you mentioned tags).

Here's what I propose:

Change -[QSObject stringValue] to add:

id handler = [self handlerForType:[self primaryType] selector:@selector(stringValueForObject:)];
if (handler) {
stringValue = [handler stringValueForObject:self];
}

That way, any object handler (including plugins) can decide what to return for the 'string value' for a given object type on a case by case basis. This is arguably clearer than having plugin devs remember to set [obj setObject:myString forType:QSTextType] on every object.

I'm working on implementing this to see how it runs

n8henrie commented 2 months ago

It doesn't seem like this is going anywhere, some I'm going to close for now -- can reopen at any time.