Change a4fea05a43b8fe57b9f43afeb6ed6bd88601c053, Reduce verbosity while reloading

[Milian Wolff]

On Wednesday 15 August 2012 08:07:29 Andreas Pakulat wrote:
> 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.

Btw, I think we should remember this issue and bring it up at the joint 
KWrite/KDevelop hack in October. I bet Dominik and Christoph will help us out.

Cheers

-- 
Milian Wolff
mail at milianw.de
http://milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120816/f91d0015/attachment.sig>