[Geany-Devel] Code review for feature #683

[Steven VALSESIA]

> 1. is it necessary to remove the reference to gthread-2.0 in the
> makefile for your changes? If not then make that a separate change
> with its justification. Never hide unrelated changes inside other
> changes.

Ok, I'll take care of that in the future.

> 2. Since you have added a preference setting it needs to be documented.

I'll check the doc to know how to doc the doc, because I don't know what
you're talking about :)

> The method suggested by Dimitar will still block, but not until the UI
> has become idle, so hopefully it will be less apparent to users.

A plugin_idle_add(func) won't create latency ?
Maybe I can create a gtk_thread ?

See you soon :)


2014-03-01 12:24 GMT+01:00 Lex Trotman <elextr at gmail.com>:

> On 1 March 2014 22:16, Steven VALSESIA <steven.valsesia at gmail.com> wrote:
> >> First, the "editor-notify" signal is a tight spot, all Scintilla
> >> events go there. Blocking them while saving may not be a good idea,
> >> especially considering that a file save error may display a modal
> >> dialog IIRC. You can do a plugin_idle_add(func) instead, and make
> >> func() return FALSE.
> >
> > I think I don't understand, where do I block these signals ?
> > My implementation of editor-notify() return FALSE, so it doesn't block
> > anything, no ? :)
>
> What Dimitar is referring to is that saving a file is a slow
> operation, and it will block the UI while it happens since your
> handler calls the save operation synchronously.  And since this is a
> "save on unfocus" that means the user moved the cursor somewhere
> unrelated and that is now possibly being blocked.  Some people insist
> on editing large files over slow remote links :( and so we can't
> always assume that file saving is instantaneous.
>
> The method suggested by Dimitar will still block, but not until the UI
> has become idle, so hopefully it will be less apparent to users.
>
> Cheers
> Lex
>
> >
> >> Second, mentioning -lgthread-2.0 twice may be required, depending on
> >> the libraries and linker, and won't do any harm otherwise.
> >
> > Ok, roger that !
> >
> >
> > 2014-03-01 12:01 GMT+01:00 Dimitar Zhekov <dimitar.zhekov at gmail.com>:
> >
> >> On Sat, 1 Mar 2014 11:28:13 +0100
> >> Steven VALSESIA <steven.valsesia at gmail.com> wrote:
> >>
> >> > Hi everybody !
> >>
> >> Hi.
> >>
> >> > Please, take a look to my patch concerning the feature request #683.
> >>
> >> A disclaimer first: I'm not using the autosave actions plugin.
> >> The code seems to match autosave closely, with no obvious errors.
> >>
> >> > Let me know if you see how I can improve my code :)
> >>
> >> First, the "editor-notify" signal is a tight spot, all Scintilla
> >> events go there. Blocking them while saving may not be a good idea,
> >> especially considering that a file save error may display a modal
> >> dialog IIRC. You can do a plugin_idle_add(func) instead, and make
> >> func() return FALSE.
> >>
> >> Second, mentioning -lgthread-2.0 twice may be required, depending on
> >> the libraries and linker, and won't do any harm otherwise.
> >>
> >> --
> >> E-gards: Jimmy
> >> _______________________________________________
> >> Devel mailing list
> >> Devel at lists.geany.org
> >> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
> >
> >
> >
> > _______________________________________________
> > Devel mailing list
> > Devel at lists.geany.org
> > https://lists.geany.org/cgi-bin/mailman/listinfo/devel
> >
> _______________________________________________
> Devel mailing list
> Devel at lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.geany.org/pipermail/devel/attachments/20140301/de73ffef/attachment-0001.html>