ext Henrik Abelsson wrote: > 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.
Thanks for the new patch, however, next time please amend your previous patch, as it's a bit confusing to look at the changes on top of the last patch. You can do this with 'git commit --amend' and then force-push it (after making sure you're not force-pushing anything into oblivion). It seems like you didn't understand what I was trying to say though. My problem with the patch was that it duplicated logic across all IFile implementations, that I think can be implemented only in the FileManager. At the moment you still have the isModified() check in all modified() methods, even though it is part of the IFile API and could be checked from FileManager. However, I see the problem of duplication is already there, and your current approach fits very nicely into the way things are done at the moment. So I think you should keep it as it is for now. :-) > 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. Please see TabSettings::m_tabKeyBehavior for a similar case. The only difference is that this enum has values assigned explicitly, which I think is good practice when the values are important. If you fix that, and squash your commits, I think you could attempt a merge request! (The easiest way I know to squash commits is to do a 'git rebase -i origin/master', which will fire up an editor with the list of commits you're going to rebase that also allows you to squash them. A less interactive way is 'git merge --squash origin/master'.) Keep in mind that the commit message should have a short subject-line (max 50-70 chars), followed by an empty line, and then any details. Your last commit's subject line was a bit long. :-) Regards, Bjørn -- Thorbjørn Lindeijer Software Engineer Nokia, Qt Software _______________________________________________ Qt-creator mailing list Qt-creator@trolltech.com http://lists.trolltech.com/mailman/listinfo/qt-creator
