Fixed launch url composition from launch configuration server, path and arguments
ClosedPublic

Authored by santilin on Oct 31 2017, 11:38 AM.

Diff Detail

Repository
R67 KDevelop Execute Browser
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
santilin created this revision.Oct 31 2017, 11:38 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptOct 31 2017, 11:38 AM
apol added a subscriber: apol.Oct 31 2017, 11:45 AM
apol added inline comments.
executebrowserplugin.cpp
87

Looks like something will be missing now, at least it won't be very backwards compatible. For "https://kde.org/index.html" the host used to be "kde.org", now you are assuming that the schema is part of the host? Should we change the serverEntry definition to be the base url rather than the hostname?

I have replied to @apol but I don't know how to submit the coment. I have 1 unsubmitted comment, but no way to submit it :(

executebrowserplugin.cpp
87

Yes, that was something odd with the old code. It added manually a fixed scheme part with url.setScheme("http"); so i guess you could not use https:// for your project.

Now, I assume the schema part is part of the host. Maybe we could change the label in the .ui

I wonder if with the old code you could open a url like "https://kde.org/index.html", I bet that you could''t.

apol added a comment.Nov 3 2017, 1:25 PM

you did fine ;)

executebrowserplugin.cpp
87

Probably not. Now if we need to go this way, it would probably make sense to use a different configuration setting, otherwise people will get weird URLs.

zhigalin added inline comments.
executebrowserplugin.cpp
87

What about

if( !host.contains(QStringLiteral("://")) )
{
    host.prepend(QStringLiteral("http://"));
}

@santilin It seems you did a duplicate here, please close D8555

zhigalin accepted this revision.Nov 8 2017, 7:40 AM

@santilin
I would like to merge it, including schema defaulting to http:// as I have proposed,
any objections?

This revision is now accepted and ready to land.Nov 8 2017, 7:40 AM
Closed by commit R67:f215abcf7461: Fixed launch url composition from launch configuration server, path and… (authored by Santiago Capel Torres <santi@holamundo.me>, committed by zhigalin). · Explain WhyJan 30 2018, 12:15 PM
This revision was automatically updated to reflect the committed changes.