D9344: [KDevelop] : consistent use of the project name (WIP)
noreply at phabricator.kde.org
Wed Apr 4 10:21:16 UTC 2018
mwolff added a comment.
In D9344#239331 <https://phabricator.kde.org/D9344#239331>, @rjvbb wrote:
> >> projectcontroller.cpp:443
> > I don't get this change, can you explain? the old code checks whether the profileFileUrl (which should *always* ends on `.kdev4`, no?) exists. In that case, we want to ask the user if he wants to override, except if the project file is equal to what we'd write out anyways.
> "Except", are you sure about that? I'm pretty certain I got the override dialog when I forced a new import from, say, the project's CMake file, and <dirname>.kdev4 existed already.
Ah, yes - that could be it. Then we should probably change the code to first construct a projectFileUrl that actually points to a .kdev4 file, and then use that in the conditional. And add comments that explain what is going on here.
>> your change seems to completely break this, as isKDevProject should always be true, and then shouldAsk always false?
> Of course I've already half forgotten why I had to make this change, more than 3 months ago. Did you read the observations I posted in comments?
Yes, but it didn't answer my question. Your comment above does though, so make sure to add that to the code too to simplify the understanding from others.
> I think that this modifies the "we want to ask the user if he wants to override, except if the project file is equal to what we'd write out anyways" to include user-selected .kdev4 filename. Getting the project filename to be written under a name that reflects the actual project name was relatively easy, but I got the override dialog when reopening the project from that file afterwards. That shouldn't happen of course, according to me at least.
Neither according to me, which makes me wonder why the `equalProjectFile` code is apparently broken?
> The purpose of this change is to give user control over the project.kdev4 file name, and one reason to do that is that it allows to create different projects in a single source tree, which can be opened concurrently. In that case you shouldn't be proposing an override in general unless you're 100% certain the user might indeed be overriding (overwriting) something.
I agree in principle, I'm just trying to understand the old code and your changes.
> There's only 1 case in that context where I can imagine that an override request would make sense: when you try to recreate a project from the CMake/QMake/Make file and give it the exact same name (override means overwrite). In all other cases you'd be creating a new .kdev4 file, and then there's no need for asking the user if he wants to override an existing project. The answer to that is implicitly no ("I just told you to create a project with a different name"). The import wizard will already auto-select an existing .kdev4 file in the 2nd step even when you started the procedure by selecting a CMake/QMake/Make file. So re-selecting that file instead of the .kdev4 file should be indication enough that you don't want to use an existing project definition, no? Except when you give that project the name of an existing project, so the opposite of what you wrote above. I suppose that assumption is why I've always felt an ambiguity in the situation around project files and names.
I agree with what you write. I don't understand why you think what I wrote above is the opposite though? Simply put: Unless you open a `*.kdev4` file explicitly, we want to ensure that we don't overwrite another existing project file with *different* contents. That's what I think the existing code was trying to do.
> I've been using this modification for almost 4 months now, without ever running into unexpected behaviour. If you can think of a sequence of actions that will no longer produce the intended result I can test of course.
To: rjvbb, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel, antismap, iodelay, vbspam, njensen, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the KDevelop-devel