[Geany-devel] HTML/XML autoindentation - Re: [PATCH] Some editor/snippets/keybindings enhancements

[Eugene Arshinov]

On Mon, 25 Oct 2010 18:01:25 +0100%
Nick Treleaven <nick.treleaven at btinternet.com> wrote:

> On Fri, 8 Oct 2010 06:03:33 +0400
> Eugene Arshinov <earshinov at gmail.com> wrote:
> 
> > > > The drawback of the third patch is that it's not completed.  If
> > > > user likes to leave HTML tags like <br> "unclosed", she would be
> > > > disturbed by automatic indentation caused by my patch, so a
> > > > check box in Preferences is desirable.  I'll code it as soon as
> > > > we decide this patch can go to trunk.
> > > 
> > > For HTML perhaps we could have a filetype pref for this.
> > > 
> > 
> > What should it look like?  I can't name this pref "autoindent"
> > because it would be confusing if for XML and HTML it only controls
> > the indentation after XML/HTML tags, not after braces in PHP/JS
> > chunks.  If I make the pref more specific (e.g., "xml-autoindent"),
> > it won't probably be quite proper to insert such a specific member
> > to GeanyFiletype struct.
> > 
> > Maybe it's more appropriate to add a check button near "Preferences
> > > Editor > Indentation > Auto-indent mode" list?  AFAIK (never used
> > > it),
> > "Match braces" mode works only for braces languages and thus is
> > already somewhat filetype-specific.
> > 
> > For the present, I attach an updated patch which doesn't insert
> > indentation after "short" HTML tags.
> 
> OK, looks like a good solution.
> 

Do I understand correctly that you think we don't need GUI preference?
 If you do, I agree :)

> Committed patch, but I disabled autoindentation if tag autoclosing is
> enabled, because the two features don't really work well together.
> 
> E.g. typing <table> adds </table> after the cursor; what if the user
> wants to put the closing tag on a newline? They press enter and the
> closing tag is indented, which is not wanted.
> 

Agree.

> >  I also modified existing tag
> > autocompletion code so that it doesn't check for HTML tags if
> > current lexer is XML, not HTML.  I slightly modified
> > utils_find_open_xml_tag() in order to reuse it in my
> > autoindentation code.  Particularly I removed `check_tag' parameter
> > and strange condition
> > 
> > else if (! check_tag && *cur == '>')
> > 	break;
> > 
> > I'm not sure why this condition was needed there.
> 
> Was it necessary to remove it? Just checking as we should leave it
> otherwise. (Currently the change is applied).

No, technically it wasn't necessary.  The removal just allows to prune
check_tag argument and make the function and it's usage a bit simpler.

Maybe you're right, we should keep the condition, but I think we
should change it to just "*cur == '>'" (so the check_tag argument
still can be removed).

The condition may become true only for invalid HTML: it should contain
two > > without < between them (follow the code of
utils_find_open_xml_tag).  The condition guarantees that if we have
such an erroneous HTML line, no close tag will be automatically
inserted.  For example, it is helpful when you write "<script>if (a >"
with XML tag autocompletion turned on.

In previous email I wrote this condition is strange as I can't
understand why we account `check_tag' argument here (i.e., why not
just check for *cur == '>').  Basically, in original editor.c:handle_xml
check_tag was set to

  0 if we autocomplete after >
  1 if we autocomplete after </

What's the difference for the condition we discuss, if we always start
backward search for an open tag _before_ those ">" and "</" chunks?

By the way, the code of utils_find_open_xml_tag is quite old (committed
in 2005, rev. 4 "Initial import").

Best regards,
Eugene.