Bring "Override Project" dialog back to action.
ClosedPublic

Authored by arrowd on Feb 2 2016, 11:57 AM.

Details

Summary

Store origin URL in the project to let project managers find out from which file it was imported. Ask user if he wants to override the existing project whenhe imports new project from the build system file and there is already .kdev file for it. Without this patch I am never asked about this.

Diff Detail

Repository
R33 KDevPlatform
Branch
project_origin (branched from 5.0)
Lint
No Linters Available
Unit
No Unit Test Coverage
arrowd updated this revision to Diff 2168.Feb 2 2016, 11:57 AM
arrowd retitled this revision from to Bring "Override Project" dialog back to action..
arrowd updated this object.
arrowd edited the test plan for this revision. (Show Details)
arrowd added a reviewer: KDevelop.
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptFeb 2 2016, 11:57 AM
mwolff added a subscriber: mwolff.Feb 2 2016, 3:07 PM
mwolff added inline comments.
shell/openprojectdialog.cpp
126

add newline after

217

const, the other methods should also be const (separate patch)

shell/openprojectdialog.h
41

origin? of what? what does this url point to? the projectUrl() maybe?

shell/projectcontroller.cpp
340

why write it to disk? if this is the project url as I think it is, then it equals the position in the file system and should not be hardcoded. The project may be moved to another place, or this file checked into VCS and used on a different machine

407

I don't get this part - you say you never saw the dialog before, yet restrict it even more with this patch. how can this trigger the dialog from showing up *more*?

kfunk requested changes to this revision.Feb 2 2016, 10:49 PM
kfunk added a reviewer: kfunk.
This revision now requires changes to proceed.Feb 2 2016, 10:49 PM
arrowd added inline comments.Feb 3 2016, 8:46 AM
shell/openprojectdialog.h
41

Origin of the project file. projectFileUrl() returns a ".kdev", while originUrl() returns, for example, "CMakeLists.txt" from which this project was created.

I thought origin was right word for this.

shell/projectcontroller.cpp
340

To let project managers find out from what file the project was created. For Haskell, for instance, there could be "project.cabal" or "stack.yaml" and they can be located in the same dir. Without this haskell project manager would be unable to figure out how to handle this project - in cabal way, or stack way.

I get your point regarding relocating the project. I guess, it is fine to store relative filename of the origin.

407

The thing is projectFileUrl always contains a ".kdev". So even if i select "CMakeLists.txt" in the "Open Project" dialog, it picks up existing ".kdev".

From my POV if user wanted to open existing project, he'd select ".kdev" in first place. But if he deliberately selects build system file it is better to ask him what's on his mind.

arrowd updated this revision to Diff 2186.Feb 4 2016, 9:47 AM
arrowd edited edge metadata.
arrowd marked an inline comment as done.

Address comments.

arrowd marked an inline comment as done.Feb 4 2016, 9:48 AM
kfunk added a comment.Feb 11 2016, 9:50 AM

@mwolff: Opinions about this one?

I kind of agree: We don't have a way to select a different project manager after we've imported the project once right now. This patch would address this.

mwolff added inline comments.Feb 12 2016, 12:18 AM
shell/openprojectdialog.h
37 ↗(On Diff #2186)

maybe use selectedUrl? definitely add a comment to explain what this returns and what the other methods return please

shell/projectcontroller.cpp
340 ↗(On Diff #2186)

yes, definitely make it relative then and also add the above as justification in a comment please

405 ↗(On Diff #2186)

just to make sure: if I select a folder, and not a file in there, will I get an annoying dialog now, or will it work as before?

arrowd added inline comments.Feb 12 2016, 8:19 AM
shell/projectcontroller.cpp
405 ↗(On Diff #2186)

Hum. At least on Windows, i can't select a folder in "Open Project" dialog. Are you able to do this on Linux?

arrowd updated this revision to Diff 2287.Feb 12 2016, 8:37 AM
arrowd edited edge metadata.

Address comments.

arrowd added inline comments.Feb 12 2016, 2:05 PM
shell/projectcontroller.cpp
407

Now i updated to last revision and was able to use a folder. Yes, the dialog pops out in this case. You want it gone?

mwolff added inline comments.Feb 14 2016, 10:24 AM
shell/projectcontroller.cpp
407

Yes, if I reopen a folder, it just should reuse the existing .kdev4 file (if available). This brings me to another question:

What happens to your code with the "origin" url when a user opens a .kdev4 file? It should just open the project as-is and not change the origin url or name or anything like that.

arrowd added inline comments.Feb 20 2016, 7:47 PM
shell/projectcontroller.cpp
407

Hum, i'm confused with that folder selection. When i initially select a folder, it can't be used to open a project:

If i enter it, the "Next" button becomes active:

Now if i get back it is still active and when i click it, it prompts me to create a project in root dir using generic project manager:

I'm not sure what i should do with this.

What happens to your code with the "origin" url when a user opens a .kdev4 file? It should just open the project as-is and not change the origin url or name or anything like that.

Yup, that's how it works. That origin field is filled only during creation of the .kdev file (that is, when we open !.kdev file).

This sounds like a bug that exists even without your changes, right? But at least right now, when you open a folder again, it will look for the .kdev4 in it (or?) and use that to open the file. If that is not the case, then it's another bug that must be fixed :)

arrowd marked 2 inline comments as done.Feb 21 2016, 9:18 AM
In D893#18992, @mwolff wrote:

This sounds like a bug that exists even without your changes, right? But at least right now, when you open a folder again, it will look for the .kdev4 in it (or?) and use that to open the file. If that is not the case, then it's another bug that must be fixed :)

When i enter a folder, it automatically selects .kdev file if it is in there. My patch didn't even touched this.
When i select a folder, i get behavior from my post above. I guess, it is not related with my patch.

mwolff accepted this revision.Mar 5 2016, 1:24 PM
mwolff added a reviewer: mwolff.

Ok, go for it. if it breaks anything I'll have to dive into the code and look at it myself. Sorry for blocking you so long!