[patches to kdelibs/base] request for comment

[Thiago Macieira]

Marcel Partap wrote:
>+bool KUrl::hasSubDomain() const
>+{
>+  if (hasHost() && host().count( QRegExp( "\\." ) ) > 1 )
>+    return true;
>+  return false;
>+}
>+
> QString KUrl::url( AdjustPathOption trailing ) const
> {
>   if ( trailing == AddTrailingSlash && !path().endsWith(
> QLatin1Char('/') ) ) { @@ -1294,7 +1301,11 @@
>   {
>      KUrl u(*this);
> 
>-     u.cd("../");
>+     if (u.hasPath() && u.path() == "/" && u.hasSubDomain())
>+        // remove subdomain up to first dot plus the dot from hostname
>+        u.setHost( u.host().remove( 0, u.host().indexOf( '.' ) + 1 ) );
>+     else
>+        u.cd("../");
> 
>      return u;
>   }

I have one concern and one issue with the code.

The concern is whether this should be in KUrl. I don't like stuffing API 
like goToFirstMondayOfNextMonth() to our classes. The use-cases are very 
limited. And those are best accomplished by external functions (the class 
members should protect the invariant). The way I see it, going up in 
subdomain is very specific to a browser, so it would belong in libkonq, 
not libkdecore.

The issue with the code is the use of QRegExp. Don't use it if you can 
avoid it. You could rewrite the hasSubDomain() function simply as:
  return host().count(QLatin1Char('.')) > 1;

Then there's your "if" line. What happens if the URL doesn't have a path 
(i.e., empty path)? Should it not go up the subdomain as well?

-- 
  Thiago Macieira  -  thiago (AT) macieira.info - thiago (AT) kde.org
    PGP/GPG: 0x6EF45358; fingerprint:
    E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080211/ea17aa6a/attachment.sig>