Created attachment 286270 [details] [review]
sourceview: compute auto-indentation text in signal handler.

This allows for consumers of GtkSourceView to implement
GtkSourceView::compute-indentation and override the default behavior.
This is useful for providing language specific indentation rules.

Since there is no padding in GtkSourceView, no class vfunc is provided.
The first handler simply wins.

nice. We had this in the todo list for some time already since we wanted to use it in the code assistance. Maybe we should unify the indentation code in some library or something?

Note that the same effect can be achieved as we do in gca [1][2], by connecting to event-after and handling key-presses after they have been handled by gsv. Although I think the signal you propose is much cleaner already, but it's not a universal solution for smart-indenting. If we are going to add more API to gsv for this kind of thing, it might be worth thinking about the larger solution (i.e. smart-indenting) so we can keep the API clean.

[1] https://git.gnome.org/browse/gedit-code-assistance/tree/src/gca-view.vala#n313
[2] https://git.gnome.org/browse/gedit-code-assistance/tree/indentbackends/c/gca-c-backend.vala

Review of attachment 286270 [details] [review]:

::: gtksourceview/gtksourceview.c
@@ +601,3 @@
 
+	/**
+	 * GtkSourceView::compute-indentation:

I'm not sure this is the best name, it's specific to auto-indent. Maybe call it query-auto-indent?

@@ +622,3 @@
+			      g_signal_accumulator_first_wins,
+			      NULL,
+	 * This will only be emitted if GtkSourceView:auto-indent is set to

Is this best practice now? I would still go with generated marshals from a list, to avoid going through libffi to emit signals.

Review of attachment 286270 [details] [review]:

::: gtksourceview/gtksourceview.c
@@ +601,3 @@
 
+	/**
+	 * GtkSourceView::compute-indentation:

"query-auto-indent" sounds find by me. Feels similar to say "query-tooltip".

@@ +622,3 @@
+			      g_signal_accumulator_first_wins,
+			      NULL,
+			      g_cclosure_marshal_generic,

The one above it passes NULL for the marshaler, I thought I'd be more specific. But in both cases I believe generic is used. I'm okay with generating a marshaler though.

Created attachment 286272 [details] [review]
sourceview: add ::query-auto-indent signal to compute indentation text

This allows for consumers of GtkSourceView to implement custom
indentation routines to override the default auto-indent behavior.
This is useful for providing language specific indentation rules.

Since there is no padding in GtkSourceView, no class vfunc is provided.
The first handler wins.

Review of attachment 286270 [details] [review]:

::: gtksourceview/gtksourceview.c
@@ +622,3 @@
+			      g_signal_accumulator_first_wins,
+			      NULL,
+	 * This will only be emitted if GtkSourceView:auto-indent is set to

Ah ok! didn't know that, I thought actually that NULL meant the default empty arg/retval marshaller :) I guess it's fine then to leave it as is.

Jesse, Do you think anything additional is needed to be able to do custom language support?

I think it makes sense to have a library that can do more of the language-specific features so that code can be shared, but in particular I'm curious if you have a suggestion based on implementing gedit-code-assistance what other stuff would be helpful here.

I certainly wouldn't mind something similar to GtkSourceLanguage for this in GSV either.

I've replaced all the generated marshallers by NULL to use the generic marshaller, it's simpler. Creating a new signal is already painful enough.

We have thought about smart-indenting in gsv for quite some time, but never settled on a satisfactory solution. Ideally I wanted to be able to specify indentation rules in the lang files, similar to how other editors do this (like sublime). @nacho knows more about this, since he tried implementing it :) In the end however, in gca we went more with the vim way, i.e. not doing it declaratively. I think it works quite well, and might actually be easier/more flexible in the end than writing regex rules.

We could think about merging the gca indent backends into gsv in the next cycle, it shouldn't be too much work to do so. Maybe @nacho can comment more on the details, it's his work.

In my opinion gsv is definetly the right place. And I still think the vim way is the "right" way at least for some languages. One way could be to have a generic regex backend which would use the language regex rules, and for other languages where we need triggers etc to have the specific backend as we do right now in gsv.

The main problem here is the pain that is to write everything in C, though we did it for gca in vala so it should be feasible.

We could also think about using libpeas in gsv.

It's easier to first have an internal API with one backend for C-like languages.
If people want to write external backend (with the API, or in .lang files), we can think about that later, and in the meantime there can be several iterations on the internal API if needed.

What do you all think about using this "query-auto-indent" signal, and then in the default handler hook that up to an internal implementation for C like languages?

That would allow us to iterate on the implementation, while still allowing for external use in the mean time. In particularly, that would let me play with things in Builder easier. (Easier for me to have people test Builder if it doesn't require jhbuild, although I guess we are probably past that point now).

Sounds good.

The list of supported languages would be hardcoded, so only certain GtkSourceLanguage's would enable the custom auto-indent.

Created attachment 286325 [details] [review]
sourceview: add ::query-auto-indent signal to compute indentation text

This allows for consumers of GtkSourceView to implement custom
indentation routines to override the default auto-indent behavior.
This is useful for providing language specific indentation rules.

Since there is no padding in GtkSourceView, no class vfunc is provided.
The first handler wins.

I just tried wiring this up. Looks like connect_after() is needed since otherwise the default handler will get called. Too bad we don't have padding in GtkSourceView for a class vfunc.

One of the other things it would be nice to do is to recompute the indent after trigger characters. For example:

  if (foo)
    {
      item1();
      item2();
      }

      ^ this should trigger recomputation and unindent 2.

My original idea for this was something along these lines:

 - a GtkSourceIndenter interface
 - a GtkSourceCIndenter class that implements the interface for C like languages
 - eventually other indenter classes (e.g. a generic one based on regexes)
 - (maybe) factor out the current code that indents to the same level into a DummmyIndenter (this is not really needed, but the refactoring would help us validate the api)
 - GtkSourceView would expose API to register an indenter
 - GtkSourceView would call the indenter methods when inserting a new line
 - lang files would use the metadata section to define their indenter
 - lang files would use the metadata section to define extra triggers

This is all very hand-wavy... here are a some open questions:

 - should the methods of the interface manipulate the buffer or should they return an indentation level and let the caller to the actual modification?
 - is defining triggers statically in the lang files enough or do we need an api to registers triggers dynamically

FWIW indentation support is the oldest open bug against GSV :)

See bug #107044 for the paleolithic discussion

for reference I agree with pbor:

currently in gca we return a IndentLevel value and let the caller to create the string out of it and delete and insert that level.

See the interface:
https://git.gnome.org/browse/gedit-code-assistance/tree/src/gca-indent-backend.vala

Basically the indent backend just implements the 2 methods:
get_triggers: which should be added to the lang files
get_indent: which is the one that returns the indent level (indent + alignment)

Apart from this, each indent backend provides a ini file with pointing out the languages that it supports. This would also be implemented in the lang file if this ever gets into gsv.

(In reply to comment #18)
> One of the other things it would be nice to do is to recompute the indent after
> trigger characters. For example:
> 
>   if (foo)
>     {
>       item1();
>       item2();
>       }
> 
>       ^ this should trigger recomputation and unindent 2.

See the gca stuff, it deals with this case. Basically we provide a set of triggers which recompute the indentation.

Another topic to take into account which gca does not deal with yet is the indentation styles. For this I have some ideas but that can be discussed later on if we ever get this in gsv.

*** Bug 107044 has been marked as a duplicate of this bug. ***

Moved to https://gitlab.gnome.org/GNOME/gtksourceview/-/issues/166