Created attachment 304385 [details] [review]
completion: close environment in a different user action

In this way, a user only interested in inserting the "\begin" part
can easily remove the "\end" by ctrl-z.

Review of attachment 304385 [details] [review]:

I agree with the proposal, but the patch needs a bit more work.

::: src/completion.vala
@@ +577,3 @@
             doc.insert (ref iter, "}", -1);
 
+        doc.end_user_action ();

The first begin_user_action() is not in the same function. So it's quite strange to end the user action here. It supposes that the caller has already called begin_user_action(). I prefer to have self-contained functions, that don't rely on things that the caller has already done.

So close_environment() should probably be split in two. For example ensure_bracket_is_closed() and close_environment(). In close_environment() we can have the begin/end_user_action. The caller would look like:

- begin_user_action()
- some inserts
- ensure_bracket_is_closed()
- end_user_action()
- close_environment() (which calls internally the begin/end_user_action)

Created attachment 304538 [details] [review]
completion: close environment in a different user action

In this way, a user only interested in inserting the "\begin" part
can easily remove the "\end" by ctrl-z.

All the begin/end_user_action() are now in the calling function, so no more hidden assumptions.

As for ensuring the closed bracket, I can move it to a separate function if you prefer, but maybe it's not worth doing (it's used only here).

Review of attachment 304538 [details] [review]:

begin/end_user_action can be nested. So it's better to have them inside close_environment(), to group the actions.

You should also explain why there is two user actions in the code, not just in the commit message.

(In reply to Sébastien Wilmet from comment #5)
> Review of attachment 304538 [details] [review] [review]:
> 
> begin/end_user_action can be nested. So it's better to have them inside
> close_environment(), to group the actions.
> 

Sorry, I don't understand this. According to gtk+ documentation:
"GtkTextBuffer maintains a count of calls to gtk_text_buffer_begin_user_action() that have not been closed with a call to gtk_text_buffer_end_user_action(), and emits the “begin-user-action” and “end-user-action” signals only for the outermost pair of calls."




> You should also explain why there is two user actions in the code, not just
> in the commit message.

Ok

begin/end_user_action can be nested, for example:

- begin_user_action
- insert/delete text
- call close_environment(), which itself calls begin/end_user_action
- end_user_action

With the above example, you can combine user actions into bigger user actions.

But it's important that if you call close_environment() alone, it's a user action in itself, so that the caller doesn't need to surround the call of close_environment() with begin/end_user_action.

Created attachment 304850 [details] [review]
completion: close environment in a different user action

In this way, a user only interested in inserting the "\begin" part
can easily remove the "\end" by ctrl-z.

Pushed:
https://git.gnome.org/browse/latexila/commit/?id=666bff0e71bb0083f4ab95b8f658bae32f8e77e7

Thanks!