[Qt-creator] Disabling File Changed dialog

[Henrik Abelsson]

2009/7/7 Thorbjørn Lindeijer <thorbjorn.lindeijer at nokia.com>

>
> Well, you're not breaking any fundamental design principles, but I do
> have some comments. :)
>

Excellent. And good comments too, thank you. :)


>
> It's not really nice that you have to do the "if (!isModified() &&
> Core::EditorManager::instance()->alwaysReload())" check in every
> editor's modified() method. Personally, I think you could put this check
> in FileManager::checkForReload(), and use a temporary IFile::ReloadAll
> behaviour (as done in EditorManager::revertToSaved()), to make each
> unmodified editor reload without asking questions.
>

Righto. That is indeed a better spot. Had I found
FileManager::checkForReload() on my own, I'd have put it in there. :)

The updated patch behavior works by setting the initial value to what was
specified in the config dialog. If the user choses a "for all" dialog option
for some file, that will apply to all modifications in that batch and
override the config dialog setting. I think that is what you want, but I'm a
bit unsure.


> When the above works out, I think the option should probably move into
> FileManager instead of EditorManager, but it's not that important. I see
> the FileManager doesn't have its own settings yet.
>

I've left it in EditorManager for the moment.



>
> An accessor like EditorManager::alwaysReload() should be const.
>


Yes, of course. (somewhat of a d'oh, how did I forget that? )


>
> By not using a bool but an enum, you could make even more people happy.
> I've read requests for never asking for reload as well, so what about
> having a combo box showing three options?
>
>  Reload externally modified files:
>    * Always ask
>    * Ask only with unsaved changes
>    * Never reload
>
> I'm not entirely sure whether these options are immediately clear
> though. And at the moment I don't think we should have a "Without
> asking" option, since I consider that quite dangerous. I'm not sure what
> the use-case is of editing a file in Qt Creator but then wanting those
> changes to be thrown away without asking.
>

Indeed. An enum is a better idea. I completely agree that a "Reload modified
without asking" is too dangerous to be useful.

I'm doing some implicit conversations between ints and the
IFile::ReloadBehavior enum in this patch, for example when storing it in the
config system. As far as I know, an enum is an integral type in C++98 and 03
and the first element will have a value of zero and the following values
will have succeedly increasing values which are exactly the guarantees I
depend on. Nonetheless, it might feel a bit dirty (it creates some gotchas
if you want to modify IFile::ReloadBehavior). Let me know if the house style
prefers explicit handling of the values.


> That's all. Looking forward to a revised patch. :-)


Here it is:

http://qt.gitorious.org/~abelsson/qt-creator/abelssons-clone/commit/21abdba6290bc671fa965a092b1f6c4b55d6d997

-henrik
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.qt-project.org/pipermail/qt-creator-old/attachments/20090707/b2ff0c72/attachment.html