Review Request 128032: Fix command line argument handling

[Urs Wolfer]

> On May 29, 2016, 9 a.m., Urs Wolfer wrote:
> > main.cpp, line 119
> > <https://git.reviewboard.kde.org/r/128032/diff/3/?file=467218#file467218line119>
> >
> >     This cannot be done this way. Protocol support is implemented via plugins. If you hardcode it here, the whole gain of the plugin architecture is lost.
> 
> Arno Möller wrote:
>     Thought that much.
>     Unfortunately, I have no idea how QT plugins work. How can I get a list of valid schemes?
>     Or should we just pass any URL to MainWindow::newConnection()?

Plugins are not known in that state IIRC. They are loaded in MainWindow::loadAllPlugins. I think you could just pass any url (a proper error message is shown later IIRC).


> On May 29, 2016, 9 a.m., Urs Wolfer wrote:
> > mainwindow.cpp, line 302
> > <https://git.reviewboard.kde.org/r/128032/diff/3/?file=467219#file467219line302>
> >
> >     Can you please move this to a separate change? I prefer to have one change in one commit.
> >     
> >     Also, I'm not sure if this correct. Before it returned. With your change, the result of #showDialogIfNeeded is ignored.
> 
> Arno Möller wrote:
>     Yes, it's ignored, but I don't think that it matters here.
>     If the user chose not to see the dialog again, why should we force it?
>     We definitely shouldn't destroy the just created view.

Hmm, I'm not exactly sure why it is this way. But isn't it false when the dialog got cancelled? In that case, the following code should not be exectued since user choose to cancel.


> On May 29, 2016, 9 a.m., Urs Wolfer wrote:
> > mainwindow.cpp, line 992
> > <https://git.reviewboard.kde.org/r/128032/diff/3/?file=467219#file467219line992>
> >
> >     Strange... Have you analyzed Git history to check why this was here?
> 
> Arno Möller wrote:
>     this call was added by commit 29a96a34dcbc3acfd3b7a1e3671bbb966e95f6b4, referencing REVIEW:113380

Hm, it was also there before this change, but in anthoer form. It really looks like a bug, but I'm confused that nobody else noticed it before...


- Urs


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128032/#review95966
-----------------------------------------------------------


On May 29, 2016, 8:43 a.m., Arno Möller wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128032/
> -----------------------------------------------------------
> 
> (Updated May 29, 2016, 8:43 a.m.)
> 
> 
> Review request for KDE Utils and Urs Wolfer.
> 
> 
> Repository: krdc
> 
> 
> Description
> -------
> 
> * Don't treat an URL passwd via the command line as a local file. That gives us file://rdp://<something or other>, which is garbage.
> * Don't shut down the view when the user chose not to show the preference dialog for the host
> * Also don't close the view on MainWindow::saveHostPrefs()
> 
> 
> Diffs
> -----
> 
>   main.cpp 73093f3 
>   mainwindow.cpp dd1d8a0 
> 
> Diff: https://git.reviewboard.kde.org/r/128032/diff/
> 
> 
> Testing
> -------
> 
> Invoke:
> $ krdc rdp://<your.favorite.rdp.host>
> 
> Without the 1. patch krdc converts the URL to vnc:// as seen in the windowTitle.
> With the 1. patch, but without the 2., the RDP connection is opened, but the view is closed instantly by MainWindow::saveHostPrefs(), leaving the user with a new connection tab.
> With both patches krdc works as expected.
> 
> 
> Thanks,
> 
> Arno Möller
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20160529/037a68be/attachment.html>