fixed and standardized URL creation

[skurfer]

I can’t believe this never blew up in our faces before. The initWithURL method would assign QSURLType as the primary type, but there was never anything for that type assigned to the object.

So I fixed that by assigning the URL as QSURLType and as QSTextType. The type assignment was moved outside of initWithURL to its own method so it could be used across the application.

All parts of the application that create QSObjects for URLs now use the same code to do so. This should ensure consistent behavior, eliminate the need for devs to manually (and potentially incorrectly) assign types for URLs, and simplify future updates by only having one place to change.

There are two basic scenarios this takes care of:

1. To create a QSObject from a URL string, you can call the class method URLObjectWithURL (which internally calls initWithURL which in turn calls the new assignURLTypesWithURL method). This is what most plug-ins are doing.
2. If you already have a QSObject then find out later that it should be a URL, you can call [self assignURLTypesWithURL:urlString] directly. (This is the case for the HTML parser and string sniffer.)

Other notes:

• The HTML parser and string sniffer were manually assigning the QSTextType after the fact, which is why URLs from those areas were more useful.
• initWithURL already had some understanding of what to do with mailto: addresses, so some redundant code in the string sniffer could be removed.

[pjrobertson]

Good idea. Having a new method will certainly make things easier

[skurfer]

Yeah, when looking at it, I see that even the mighty initWithURL takes a string, not a URL. We can’t very well rename that one. :) I’ll change some variable names around, but I’m not going to rename any methods. (Since the other methods in that class end with “URL” and take URL strings, I think it’s actually more consistent to leave it.)

[skurfer]

Scope creep! OK, I changed all the URLs-as-strings to urlString. I also noticed some things in the setQuickIconForObject method that could be improved.

[pjrobertson]

don'tcha just love the creep?!

Looks good :)

But... if you want even more scope creep, I've played with setting primary
types to QSEmailAddressType - to get icons to work you just need to
uncomment line 919 of QSObject.m

It goes on and on! :P

On 11 June 2011 23:58, skurfer <
reply@reply.github.com>wrote:

Scope creep! OK, I changed all the URLs-as-strings to urlString. I also
noticed some things in the setQuickIconForObject method that could be
improved.

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

[skurfer]

I’ve uncommented that line (and also changed it from QSContactEmailType to QSEmailAddressType). The former was disabled in QSTypes for some reason (and they both refer to the same string anyway).

[skurfer]

Don’t merge this yet. I found a minor annoyance that I want to fix first. ("http://" shouldn’t be appended to text the user types anywhere except in the value for QSURLType.)

[skurfer]

The smarts for deciding whether or not to prepend “http://“ have been moved to the general application-wide method. This also allows us to set a URL string only for the URL type while preserving the original string for other purposes (like paste, email to, use as remote host, etc.).

[pjrobertson]

I've pretty sure QS always used to be like this, but what if the URL should be https://?

I don't think it'd make much difference, but whilst we're on the topic it may be worth thinking about. ftp:// as well?

Finally, I remember [NSURL scheme] behaving a bit strangely for me when I was doing the stringSniffing business.
It was the main reason why define:word was turned into a URL — because [NSURL scheme] returned @"define"

Again, I don't think it'll make a difference here, since the assignURLTypeWithURL: method should only be called if we're sure the string is an URL (it shouldn't be the job of this method to check), which has been done in stringSniffing.

All looks good. I'll keep playing

[skurfer]

I've pretty sure QS always used to be like this, but what if the URL should be https://?

Yes, it typically worked this way. I broke it, but have fixed it now. It only adds http if there’s no scheme, so if it should have https, I imagine it already will and nothing will happen. Same with ftp. The behavior is the same as before, I just took it from sniffString and put it in a central, re-usable place.

Finally, I remember [NSURL scheme] behaving a bit strangely for me when I was doing the stringSniffing business.

Hmm. It wasn’t well documented, but I saw it in sniffString and thought it was appropriate to use. If I think about what will happen in practice, I believe it’s fine (of not desirable) to use it this way. That is to say, if an “invalid” scheme is found, the value stored for the URL type just won’t get “http://” added to it. It’ll err on the side of not screwing with the input. And the value stored for the text type is unaffected either way.

Again, I don't think it'll make a difference here, since the assignURLTypeWithURL: method should only be called if we're sure the string is an URL (it shouldn't be the job of this method to check), which has been done in stringSniffing.

I don’t think we should rely on what sniffString checks since this is meant to be a general purpose method, but from what I described above, I think it’s fine.

One related (very old) issue remains: Something in the clipboard history that looks like a hostname has “http://” prepended to it, so when you paste, you don’t actually get what was copied. I haven’t looked into that yet, but don’t expect it to be part of this request.

[pjrobertson]

Fine with all of the above. It was mainly just for clarification and to make
sure nothing had been forgotten.

One related (very old) issue remains: Something in the clipboard history
that looks like a hostname has “http://” prepended to it, so when you paste,
you don’t actually get what was copied. I haven’t looked into that yet, but
don’t expect it to be part of this request.

I'm not having this problem?
I'm copying google.com using ⌘C then in the clipboard viewer it's being
displayed as google.com and if I paste I get google.com.

Are you saying that when you paste you get http://google.com ?

*P.S. I'm using a modded clipboard plugin. If the above is correct I'll
check with the old clipboard plugin

On 14 June 2011 13:22, skurfer <
reply@reply.github.com>wrote:

I've pretty sure QS always used to be like this, but what if the URL
should be https://?

Yes, it typically worked this way. I broke it, but have fixed it now. It
only adds http if there’s no scheme, so if it should have https, I imagine
it already will and nothing will happen. Same with ftp. The behavior is the
same as before, I just took it from sniffString and put it in a central,
re-usable place.

Finally, I remember [NSURL scheme] behaving a bit strangely for me when
I was doing the stringSniffing business.

Hmm. It wasn’t well documented, but I saw it in sniffString and thought
it was appropriate to use. If I think about what will happen in practice, I
believe it’s fine (of not desirable) to use it this way. That is to say, if
an “invalid” scheme is found, the value stored for the URL type just won’t
get “http://” added to it. It’ll err on the side of not screwing with
the input. And the value of stored for the text type is unaffected either
way.

Again, I don't think it'll make a difference here, since the
assignURLTypeWithURL: method should only be called if we're sure the string
is an URL (it shouldn't be the job of this method to check), which has been
done in stringSniffing.

I don’t think we should rely on what sniffString checks since this is
meant to be a general purpose method, but from what I described above, I
think it’s fine.

One related (very old) issue remains: Something in the clipboard history
that looks like a hostname has “http://” prepended to it, so when you
paste, you don’t actually get what was copied. I haven’t looked into that
yet, but don’t expect it to be part of this request.

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

[pjrobertson]

• take that back.
  If you immediately hit ⌘V after copying it's fine. But if you drag from the
  clipboard history or get a previous entry it's not. I see the problem.

This is a problem with the putOnPasteboard: method.
I have a branch where this has been heavily modified and hopefully this is
fixed there as well.
Yep, it happens in the writeObjectToPasteboard: method in
QSObject_Pasteboard.m and I've fixed that :)
http://google.com

On 14 June 2011 13:28, Patrick Robertson robertson.patrick@gmail.comwrote:

Fine with all of the above. It was mainly just

http://google.com

for clarification and to make sure nothing had been forgotten.

One related (very old) issue remains: Something in the clipboard history
that looks like a hostname has “http://” prepended to it, so when you
paste, you don’t actually get what was copied. I haven’t looked into that
yet, but don’t expect it to be part of this request.

I'm not having this problem?
I'm copying google.com using ⌘C then in the clipboard viewer it's being
displayed as google.com and if I paste I get google.com.

Are you saying that when you paste you get http://google.com ?

*P.S. I'm using a modded clipboard plugin. If the above is correct I'll
check with the old clipboard plugin

On 14 June 2011 13:22, skurfer <
reply@reply.github.com>wrote:

I've pretty sure QS always used to be like this, but what if the URL
should be https://?

Yes, it typically worked this way. I broke it, but have fixed it now. It
only adds http if there’s no scheme, so if it should have https, I imagine
it already will and nothing will happen. Same with ftp. The behavior is the
same as before, I just took it from sniffString and put it in a central,
re-usable place.

Finally, I remember [NSURL scheme] behaving a bit strangely for me
when I was doing the stringSniffing business.

Hmm. It wasn’t well documented, but I saw it in sniffString and thought
it was appropriate to use. If I think about what will happen in practice, I
believe it’s fine (of not desirable) to use it this way. That is to say, if
an “invalid” scheme is found, the value stored for the URL type just won’t
get “http://” added to it. It’ll err on the side of not screwing with
the input. And the value of stored for the text type is unaffected either
way.

Again, I don't think it'll make a difference here, since the
assignURLTypeWithURL: method should only be called if we're sure the string
is an URL (it shouldn't be the job of this method to check), which has been
done in stringSniffing.

I don’t think we should rely on what sniffString checks since this is
meant to be a general purpose method, but from what I described above, I
think it’s fine.

One related (very old) issue remains: Something in the clipboard history
that looks like a hostname has “http://” prepended to it, so when you
paste, you don’t actually get what was copied. I haven’t looked into that
yet, but don’t expect it to be part of this request.

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

[skurfer]

Yeah, the most recent thing copied is fine. It’s items in the history that get molested.

I thought I was using your modified clipboard plug-in, but maybe there have been further updates.

[pjrobertson]

Looks good.

One last thing you forgot to do was change the urlString names in the .h file

Thanks for doing that though, it definitely looks a lot better (to me at least :) )

[skurfer]

One last thing you forgot to do was change the urlString names in the .h file

Good catch. Done.

[pjrobertson]

Pssst! You changed the URLObjectWithURL, initWithURL urlString names as well ;)

8523ce6

[skurfer]

I blame it on lack of sleep. :)

[pjrobertson]

Tsk. Tell me about it! :)

[pjrobertson]

Just an FYI, here's what Apple have reported on creating an url as follows: [NSURL urlWithString:example.com:80 ]

Engineering has determined that this issue behaves as intended based on the following information:

With no scheme in your URL string, you are creating a relative reference. The rules for a relative reference (see rfc3986) look like this:

   relative-ref  = relative-part [ "?" query ] [ "#" fragment ]

   relative-part = "//" authority path-abempty
                 / path-absolute
                 / path-noscheme
                 / path-empty

Since "example.com:80" is the authority, you need to start your relative-part string with "//". With that change, this code:

    NSURL *url = [NSURL URLWithString:@"//example.com:80"];
    NSLog(@"Scheme: %@, Host: %@, Port: %@",[url scheme], [url host], [url port]);

displays:

    Scheme: (null), Host: example.com, Port: 80