On Mon, 25 Oct 2010 18:01:25 +0100% Nick Treleaven nick.treleaven@btinternet.com wrote:
On Fri, 8 Oct 2010 06:03:33 +0400 Eugene Arshinov earshinov@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.