Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-17 Thread Jan Grulich
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120606/ --- (Updated Oct. 17, 2014, 9:36 a.m.) Status -- This change has been ma

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-17 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120606/#review68597 --- Ship it! Thanks. Leaves one small issue. src/filewidgets/ku

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-17 Thread Jan Grulich
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120606/ --- (Updated Říj. 17, 2014, 7:53 dop.) Review request for KDE Frameworks and

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-17 Thread David Faure
> On Oct. 16, 2014, 2:43 p.m., David Faure wrote: > > src/filewidgets/kurlnavigator.cpp, line 717 > > > > > > This looks wrong. > > > > After this line, path can contain either a path or a URL. > >

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-17 Thread David Faure
> On Oct. 16, 2014, 2:43 p.m., David Faure wrote: > > src/filewidgets/kurlnavigator.cpp, line 717 > > > > > > This looks wrong. > > > > After this line, path can contain either a path or a URL. > >

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-17 Thread Jan Grulich
> On Říj. 16, 2014, 2:43 odp., David Faure wrote: > > src/filewidgets/kurlnavigator.cpp, line 717 > > > > > > This looks wrong. > > > > After this line, path can contain either a path or a URL. > >

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-16 Thread David Faure
> On Oct. 16, 2014, 2:09 p.m., Aleix Pol Gonzalez wrote: > > Can we maybe get a unit test for that? > > Jan Grulich wrote: > I don't think this is necessary, just take a look how it was implemented > before [1]. > > QString pathOrUrl = currentUrl.pathOrUrl(); vs QString path = > c

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-16 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120606/#review68557 --- src/filewidgets/kurlnavigator.cpp

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-16 Thread Jan Grulich
> On Říj. 16, 2014, 2:09 odp., Aleix Pol Gonzalez wrote: > > Can we maybe get a unit test for that? I don't think this is necessary, just take a look how it was implemented before [1]. QString pathOrUrl = currentUrl.pathOrUrl(); vs QString path = currentUrl.toString(QUrl::PreferLocalFile); an

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-16 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120606/#review68553 --- Can we maybe get a unit test for that? - Aleix Pol Gonzalez

Re: Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-16 Thread Lukáš Tinkl
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120606/#review68552 --- Tested, works fine - Lukáš Tinkl On Říj. 16, 2014, 3:56 odp

Review Request 120606: Properly parse URL in KUrlNavigator

2014-10-16 Thread Jan Grulich
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120606/ --- Review request for KDE Frameworks and David Faure. Repository: kio Desc