[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdelibs/kdecore/io
From:       Andreas Pakulat <apaku () gmx ! de>
Date:       2008-12-04 12:16:12
Message-ID: 20081204121612.GC2000 () morpheus ! apaku ! dnsalias ! org
[Download RAW message or body]

On 04.12.08 12:06:47, Carlo wrote:
> On Thu, Dec 4, 2008 at 10:59 AM, Andreas Pakulat <apaku@gmx.de> wrote:
> > On 04.12.08 10:30:10, Carlo wrote:
> >> On Thu, Dec 4, 2008 at 9:02 AM, Andreas Pakulat <apaku@gmx.de> wrote:
> >> > On 03.12.08 23:58:01, Carlo Segato wrote:
> >> >> SVN commit 892299 by segato:
> >> >>
> >> >> convert backslashes to forwardslashes in cleanpath, otherwise it returns an empty string
> >> >
> >> > Are you sure this is correct? If I understand this correctly I can't have a
> >> > url that uses forward-slash as path separator, but contains backslashes
> >> > anymore. Apart from that this looks like something thats win32 specific and
> >> > might rather show a bug in some other code. I don't think you're supposed
> >> > to use native paths with KUrl, as a Url is defined to always use forward
> >> > slashes as path separator.
> >> >
> >> > I think this needs to be reverted.
> >> >
> >> > Andreas
> >> >
> >> > --
> >> > Make a wish, it might come true.
> >> >
> >> yes it's win32 specific, the problem is that if I open a file from
> >> command line with kate like this "kate c:\file.txt" kate uses
> >> cleanpath that returns c:/ since it can't find any forwardslashes, I
> >> didn't tought about path that contains \ as non dir separator, and I
> >> can't think of a way to change only separators
> >
> > Well, the error is in kate, it apparently doesn't use QFile&Co to convert
> > the path read from argv. It needs to do that, so the path it hands over to
> > KUrl or anything else is converted by Qt from native path to
> > forward-slashes - properly.
> >
> > So please revert and fix kate instead.
> >
> > Andreas
> >
> > --
> > You'll wish that you had done some of the hard things when they were easier
> > to do.
> >

That kate, when taking a file from argv it should convert that from a
native path to a "qt path" (or unix) using QDir::fromNativeSeparators()
before creating a url from that.

Where exactly does kate do this? katemain.cpp uses args->url() which
returns a KUrl. 

But looking at KCmdLineArgs::url/makeURL it seems there's an error there,
because it doesn't convert the native path into a proper url-path. So thats
probably where the fix (using QDir::fromNativeSeparators) should be done.

> anyway I was looking at cleanPath in QDir and it does this
> 
> QChar dir_separator = separator();
>     if(dir_separator != QLatin1Char('/'))
> 	name.replace(dir_separator, QLatin1Char('/'));

But thats different, it will only do the replacement on windows and there
\ cannot be used in a path.
 
> we can do the same in kurl or even use #ifdef so only windows will be affected

See above, IMHO the place for the fix is simply wrong. A url _always has
"/" as path, if not the url is broken.

Andreas

-- 
An avocado-tone refrigerator would look good on your resume.
[prev in list] [next in list] [prev in thread] [next in thread]