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

Treat FQDN and IP address as a remote host #497

Merged
merged 3 commits into from Jan 31, 2012
Merged

Treat FQDN and IP address as a remote host #497

merged 3 commits into from Jan 31, 2012

Conversation

skurfer
Copy link
Member

@skurfer skurfer commented Oct 6, 2011

This assigns the “remote host” type to strings that are recognized as hostnames or IP addresses.

That type doesn’t exist without a plug-in installed, but there’s precedent for this, so I think it’s OK. (Strings beginning with = are given a type that only the Calculator plug-in can use.)

A small number of existing users might end up with the “SSH” action higher in priority than “Open URL”. They can fix this easily by dragging the actions in Preferences, but it’s still annoying. I’ve adjusted priorities so a new user with a clean install will definitely get “Open URL” above “SSH”.

The above only applies to “URLs” that are auto-detected by sniffString. Actual URLs from bookmarks, etc. are not affected. Strings that begin with a protocol are also not affected. And of course none of it is an issue if the Remote Hosts plug-in isn’t installed (which I suspect is true for most people).

@pjrobertson
Copy link
Member

pjrobertson commented Oct 7, 2011

OK your rationale seems fair enough. I have no objections to adding another string - until we come up with a better solution :)

I'll merge it then test with and without the remote hosts plugin.

This probably won't be soonish though...

@skurfer skurfer mentioned this pull request Jan 18, 2012
@pjrobertson
Copy link
Member

pjrobertson commented Jan 28, 2012

Just wondering - why couldn't the action precedence of the Remote Hosts action just be moved down, instead of moving the open URL action up?

@skurfer
Copy link
Member Author

skurfer commented Jan 28, 2012

Just wondering - why couldn't the action precedence of the Remote Hosts action just be moved down, instead of moving the open URL action up?

In hindsight, I guess it could have. This will only affect users that have never had either installed, so it could have been adjusted either place. But I was working on the app at the time, so that must have been where my head was. Plus this way, we wouldn’t have to worry about which one gets released in what order. (Not that that would be difficult.)

Is it worth changing? I think the precedence for Open URL deserves to be pretty high.

@pjrobertson
Copy link
Member

pjrobertson commented Jan 31, 2012

I'm just worried that if we change the precedence of 'Open URL' then it might mess up the precedence of some other action we've forgotten about. Unlikely, but maybe.

As long as you've tested on a new install and there doesn't seem to be a problem, then I goes it's fine doing it this way round :)

Just making sure: the other changes (to the sniffString) here are still useful aren't they? They allow you to use Remote Host actions on any URL?

@skurfer
Copy link
Member Author

skurfer commented Jan 31, 2012

I'm just worried that if we change the precedence of 'Open URL' then it might mess up the precedence of some other action we've forgotten about.

Valid concern, but I don’t see how in this case since it only applies to the URL type.

As long as you've tested on a new install and there doesn't seem to be a problem

I have. It works as expected.

Just making sure: the other changes (to the sniffString) here are still useful aren't they? They allow you to use Remote Host actions on any URL?

This has no effect on URLs in the catalog, but anything that’s typed by hand or grabbed from another app that looks like a hostname or IP address will get Remote Host actions. Other behavior previously in sniffString should still work as it did before. Does that answer your question?

pjrobertson added a commit that referenced this pull request Jan 31, 2012
Treat FQDN and IP address as a remote host
@pjrobertson pjrobertson merged commit 933b0f6 into quicksilver:master Jan 31, 2012
@pjrobertson
Copy link
Member

pjrobertson commented Jan 31, 2012

Does that answer your question?

Yep, perfect. I'll answer your pull request now...
Took some time eh?!

On 31 January 2012 19:03, Rob McBroom <
reply@reply.github.com

wrote:

I'm just worried that if we change the precedence of 'Open URL' then it
might mess up the precedence of some other action we've forgotten about.

Valid concern, but I dont see how in this case since it only applies to
the URL type.

As long as you've tested on a new install and there doesn't seem to be a
problem

I have. It works as expected.

Just making sure: the other changes (to the sniffString) here are
still useful aren't they? They allow you to use Remote Host actions on any
URL?

This has no effect on URLs in the catalog, but anything thats typed by
hand or grabbed from another app that looks like a hostname or IP address
will get Remote Host actions. Other behavior previously in sniffString
should still work as it did before. Does that answer your question?


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

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

2 participants