Change a4fea05a43b8fe57b9f43afeb6ed6bd88601c053, Reduce verbosity while reloading

[Andreas Pakulat]

Hi,

On Wed, Aug 15, 2012 at 1:17 AM, Aleix Pol <aleixpol at kde.org> wrote:
> On Tue, Aug 14, 2012 at 7:42 PM, Andreas Pakulat <apaku at gmx.de> wrote:
>> Hi,
>>
>> On Tue, Aug 14, 2012 at 7:32 PM, Andreas Pakulat <apaku at gmx.de> wrote:
>>> Hi,
>>>
>>> On Tue, Aug 14, 2012 at 1:31 PM, Aleix Pol <aleixpol at kde.org> wrote:
>>>> On Tue, Aug 14, 2012 at 8:24 AM, Andreas Pakulat <apaku at gmx.de> wrote:
>>>>> Hi,
>>>>>
>>>>> just read my irc backlog and noticed that commit (I'm not reading
>>>>> commit mails anymore). I'm wondering about two things:
>>>>>
>>>>> a) currently it looks like the reload-questions is not shown anymore
>>>>> when the document is dirty. That sounds wrong, since it would mean the
>>>>> user as unsaved changes and the document is reloaded, so he can loose
>>>>> data
>>>> It's dirty in the file-system, the flag modified in the IDE is Modified.
>>>> Also IDocument::reload is triggered when you call the reload action.
>>>>
>>>>>
>>>>> b) Is this really the right way? This function is also called when
>>>>> switching between documents, which means that with the change one
>>>>> could loose saved changes now by simply switching through tabs or so.
>>>> Switching tabs doesn't and shouldn't call IDocument::reload()
>>>> I made this patch because it doesn't make sense to me that when we
>>>> explicitly reload a file to see what changed in the file system, we
>>>> get a dialog asking if we are sure we want that.
>>>> Yes, that's why I'm reloading in the first place.
>>>
>>> After double-checking the actual code I take back my comments and
>>> agree with you. The patch is completely ok. Thanks for working on
>>> that.
>>
>> Only thing lacking  (after some more digging through the code) is that
>> File->Reload to reload just the current document will still show the
>> warning. But thats not fixable by us I guess since the action is
>> contributed by katepart.
>>
> That's correct.
> TL;DR: Patches welcome

Yeah, should be rather easy to patch that in katepart.

> Also when you change a bunch of files (for example when you git
> checkount <branch>) you'll get all files complaining that they have to
> be reloaded (which is why I wanted to improve "Reload All"). Now we
> have an alternative but I'm not sure if we still want those dialogs
> there, still. Maybe we could add a "Reload All" option to that dialog?
> (which belongs to kate, AFAIR).

I think its good that katepart went the 'safe' way of always asking
the user, this is better than blindly reloading from disk - I've had a
few cases where an accidental git checkout -f could be 'reverted' by
force-writing from the still-open editor. That being said, the
host-application needs to have more control over this as it currently
has. The git-case is a nice example, the host app knows this happens
so it can tell 'reload all documents' after its done. That being said
if we know that "Reload All" doesn't cause unsaved changes to be
dropped we could simply have the git jobs call
idocumentcontroller::reloadAll() once its added to the public API.
There is already API to save all documents in the interface, I don't
see a reason to not have reloadAll there too.

> In any case, why does KatePart even throw dialogs at us? Shouldn't be
> the host application responsible to handle those cases by providing
> all the needed information?

Thats something you should ask on the kwrite-devel list. One obvious
reason is to avoid code duplication and also avoid exposing some kind
of DialogFactory in the KTE API.

Andreas