Bug 101213 - automate typing for programming
Summary: automate typing for programming
Status: CLOSED FIXED
Alias: None
Product: kate
Classification: Applications
Component: kwrite (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-03-10 04:09 UTC by Mathieu Jobin
Modified: 2012-03-31 10:29 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
1 try at adding to this (1.26 KB, patch)
2005-07-27 20:05 UTC, Mathieu Jobin
Details
this would be my final patch (3.33 KB, patch)
2005-08-01 02:23 UTC, Mathieu Jobin
Details
same patch, just the patch header were not correct (3.41 KB, patch)
2005-08-01 03:47 UTC, Mathieu Jobin
Details
binary files for gentoo P3 (689.07 KB, application/x-tgz)
2005-08-13 07:19 UTC, Mathieu Jobin
Details
some updates, I think its better now. (3.99 KB, patch)
2005-08-14 16:44 UTC, Mathieu Jobin
Details
binary update, for the curious (689.24 KB, application/x-tgz)
2005-08-14 16:45 UTC, Mathieu Jobin
Details
KDE 3.5 compatible patch with some minor changes. (3.66 KB, patch)
2005-08-21 00:49 UTC, Dominik Haumann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Jobin 2005-03-10 04:09:44 UTC
Version:            (using KDE Devel)
Installed from:    Compiled sources

this bug goes along the kwrite part, so that every kde application gets this magic feature.

when editing code, (c, php, ruby), there is some typing we always do and get annoying .....

how many of you type 
"()" <left arrow> "code" when typing "(code)"

what if both braces get outputted when I typed the left one only, and the cursor is already place within the two ? what is the chance the user want a right end brace when typing a left one, 98% ?

what if we could just select some code press ( and get both around the selection.

sounds weird and annoying, but once you try this.... its great and you can't live without it.

every pair matching caractere could do that, ( [ { ' " 

thanks
Comment 1 Anders Lund 2005-03-10 09:13:14 UTC
We allready have an options to insert closing brackets, but it does not support enclosing a selection, and I'd be hesitant to allow that, because the default action when typing into a selection is overwrite. It would mean potentially 4 extra tests for when typing into a selection (is autobrackets on, and is the character an opening bracket). Opinions anyone?

The current autobracket function is really stupid btw, as it ALLWAYS inserts - maybe it should not do so if there is a word character to the immediate right?

The auto brackets option is in the Editing control panel, and there is a corresponding document variable "auto-brackets on|off".

The options causes insertion of a closing ({( but not < because < or > is often a operator.
Comment 2 Mathieu Jobin 2005-03-28 00:43:45 UTC
thank you for the "Auto Brackets" option, I did not know it was already in there.. now, I just have quotes and doublequotes missing ;)

about enclosing selection, its a little bit weird at the beginning, but it quickly becomes really good.

changing a == b into (a == b) by selecting the content and pressing ( is great.

for you check before overwriting selection, you got a check the content too.
If I select a doublequote, and try to enclose it with a quote, big chance what I wanted is to overwrite the doublequote by a quote, not to get something like '"'

thanks

Comment 3 Mathieu Jobin 2005-03-28 00:45:56 UTC
another thing

with autobrakets on when I press [ and get [], if I hit backspace right after, both should get deleted, not only the left one. its kinda a magic but....

however, if I move my cursor with the arrow or type anything, backspace is back to his standard behavious

Comment 4 Mathieu Jobin 2005-07-17 15:37:04 UTC
would you please point me out where this is in the code, just in case i get the courage to attack some part of it ?

....

thanks

Comment 5 Mathieu Jobin 2005-07-25 18:04:00 UTC
is it at 

bool KateDocument::typeChars ( KateView *view, const QString &chars )
"./kdelibs/kate/part/katedocument.cpp" 4963 lines --57%--                                                                                                                            2868,7        57%

Comment 6 Mathieu Jobin 2005-07-26 21:29:36 UTC
I'm loooking for ....
 - a function returning the selection.
 - a function returning the caracter next to the cursor

I want to see if I can improove this function a little bit.
and I need to act on those. how easy it is to replace the current selection by some other text ? like the selection surrounded by brackets ?

thanks
Comment 7 Mathieu Jobin 2005-07-27 18:32:58 UTC
what is the best way to try any code I wrote for this ?

recompile and install kate, close all instance, reopen it...

seems unsafe, never know if I'm actually running my latest stuff. installing test code ? sounds wrong ? not sure how best to proceed

Comment 8 Mathieu Jobin 2005-07-27 19:01:51 UTC
hopefully i can just recompil kdelibs/kate and do "god knows what" with it to test .....

Comment 9 Mathieu Jobin 2005-07-27 20:05:28 UTC
Created attachment 11952 [details]
1 try at adding to this

ok, I don't have a spare computer to test it on.... 
trying compiling and installing small part of kdelibs is, at least for me, very
difficult. so I dont know how to test (help appreciated)

so far, in this patch, i just added ' and " to the already present ({[

and I try to backup the selection, so if there is any selection, I surround the
selection by the brackets.

next step should be to check the caracter next to my cursor, there is few cases
where you don't want that code to be run. I dont know how to get the value of
the next caracter after my cursor

help appreciated

thanks
Comment 10 Mathieu Jobin 2005-07-31 07:56:43 UTC
whooo hoooo.....

I can't believe it,... my patch works, the patch I submitted in my previous comments works. ok, its a simple one, but a great improovement IMHO.

I just upgraded to KDE 3.4.2, recompiled kdelibs with my patch (without modification) compile on the first try and actually work using kate.

to test, enable Auto Bracket in the editing tab of the kate/kwrite config.

you can now select text and type ( to get your text surrounded like (text)

TODO: 

- if no selection and caracters on the right of my cursor then, don't do it.
- if I press delete and I'm in the middle of an empty () or [] or {} or "" or '' then... delete both.

we'll see after that....

Comment 11 Mathieu Jobin 2005-07-31 20:05:56 UTC
[11:01] <jowenn|amd64> kde4: eg QString myline=document->line(cursor.line()); --> myline.at(cursor().column()-1)
[11:03] <jowenn|amd64> kde3.x: eg QString myline=KtextEditor::editInterfacE(document)->textLine(cursor.line()); --> myline.at(cursor().column()-1)
[11:03] <jowenn|amd64> or -2/-3.../+1/+2/+3/
[11:03] <jowenn|amd64> ...
[11:03] <somekool> thank you very very much

Comment 12 Mathieu Jobin 2005-08-01 02:23:46 UTC
Created attachment 12027 [details]
this would be my final patch

there it is ... my final version for this bug. except if you would like me to
change anything .....

patch is against 3.4.2, and works fine on my computer.
if anyone could please test it against 3.5 that'd would be awesome
I'll do all my possible to test it myself against 4.0

please give me comments....

thanks
Comment 13 Mathieu Jobin 2005-08-01 03:47:26 UTC
Created attachment 12028 [details]
same patch, just the patch header  were not correct
Comment 14 Mathieu Jobin 2005-08-01 03:56:07 UTC
I think this answer what is asked on bug #79200

Comment 15 Mathieu Jobin 2005-08-13 07:19:21 UTC
Created attachment 12203 [details]
binary files for gentoo P3

for whom who does not want to recompil kdelibs (because its what I had to do to
test it, you can try backuping those two files from your installation and
replacing them by the two supplied in this archive.

it at least work for me, on both of my computers

gentoo P3 and P4 (compiled on P3), KDE 3.4.2

I don't know what else could make it break... anyway... its an easy try

mkdir -p ~/backup
cp /usr/kde/3.4/lib/kde3/libkatepart.* ~/backup/
tar xvzf <file>.tag.gz
cp libkatepart.* /usr/kde/3.4/lib/kde3/
Comment 16 Mathieu Jobin 2005-08-13 18:59:59 UTC
ok, the small protection I put is too restrictive,... I'll remove it or change it.

and I'll also make sure that "" does not becomes """ incase people enter both, I'll cancel the second one, so ()) and []] would not happen either.

Comment 17 Mathieu Jobin 2005-08-14 16:44:19 UTC
Created attachment 12219 [details]
some updates, I think its better now.

some updates, I think its better now.
Comment 18 Mathieu Jobin 2005-08-14 16:45:38 UTC
Created attachment 12220 [details]
binary update, for the curious
Comment 19 Dominik Haumann 2005-08-21 00:49:34 UTC
Created attachment 12289 [details]
KDE 3.5 compatible patch with some minor changes.

This patch applies to upcoming KDE 3.5. Some parts are simplified (removed all
regular expressions), but in general it's the same as proposed before.
I'm not using the autobracket feature myself, so I can not really comment
whether this is a good behaviour now.
Cullmann: I can commit it if wanted, and you take a look at akademy. We can
still remove it then. k?
Comment 20 Mathieu Jobin 2005-09-29 03:43:44 UTC
I thought this would be applied in SVN for testing and approval ? 

Comment 21 Dominik Haumann 2005-10-02 18:24:32 UTC
SVN commit 466351 by dhaumann:

if auto bracket option is on
 - try to complete "" and '' as well
 - if there is a selection, enclose the selection
 - try to guess when a corresponding completion character is not wanted

This is pretty close what was suggested in bug 101213. As it seems very few
users use auto brackets (we all don't) it is hard to get feedback. So let's
see how users react to this...

Patch from Mathieu Robin, thanks.
BUG: 101213


 M  +58 -7     katedocument.cpp  


--- branches/KDE/3.5/kdelibs/kate/part/katedocument.cpp #466350:466351
@@ -2857,19 +2857,52 @@
   bool bracketInserted = false;
   QString buf;
   QChar c;
+
   for( uint z = 0; z < chars.length(); z++ )
   {
     QChar ch = c = chars[z];
-
     if (ch.isPrint() || ch == '\t')
     {
       buf.append (ch);
 
       if (!bracketInserted && (config()->configFlags() & KateDocument::cfAutoBrackets))
       {
-        if (ch == '(') { bracketInserted = true; buf.append (')'); }
-        if (ch == '[') { bracketInserted = true; buf.append (']'); }
-        if (ch == '{') { bracketInserted = true; buf.append ('}'); }
+        QChar end_ch;
+        bool complete = true;
+        QChar prevChar = textLine->getChar(view->cursorColumn()-1);
+        QChar nextChar = textLine->getChar(view->cursorColumn());
+        switch(ch) {
+          case '(': end_ch = ')'; break;
+          case '[': end_ch = ']'; break;
+          case '{': end_ch = '}'; break;
+          case '\'':end_ch = '\'';break;
+          case '"': end_ch = '"'; break;
+          default: complete = false;
+        }
+        if (complete)
+        {
+          if (view->hasSelection())
+          { // there is a selection, enclose the selection
+            buf.append (view->selection());
+            buf.append (end_ch);
+            bracketInserted = true;
+          }
+          else
+          { // no selection, check whether we should better refuse to complete
+            if ( ( (ch == '\'' || ch == '"') &&
+                   (prevChar.isLetterOrNumber() || prevChar == ch) )
+              || nextChar.isLetterOrNumber()
+              || (nextChar == end_ch && prevChar != ch) )
+            {
+              kdDebug(13020) << "AutoBracket refused before: " << nextChar << "\n";
+            }
+            else
+            {
+              buf.append (end_ch);
+              bracketInserted = true;
+            }
+          }
+        }
       }
     }
   }
@@ -2998,13 +3031,31 @@
   if ((col == 0) && (line == 0))
     return;
 
+  int complement = 0;
   if (col > 0)
   {
+    if (config()->configFlags() & KateDocument::cfAutoBrackets)
+    {
+      // if inside empty (), {}, [], '', "" delete both
+      KateTextLine::Ptr tl = m_buffer->plainLine(line);
+      if(!tl) return;
+      QChar prevChar = tl->getChar(col-1);
+      QChar nextChar = tl->getChar(col);
+
+      if ( (prevChar == '"' && nextChar == '"') ||
+           (prevChar == '\'' && nextChar == '\'') ||
+           (prevChar == '(' && nextChar == ')') ||
+           (prevChar == '[' && nextChar == ']') ||
+           (prevChar == '{' && nextChar == '}') )
+      {
+        complement = 1;
+      }
+    }
     if (!(config()->configFlags() & KateDocument::cfBackspaceIndents))
     {
       // ordinary backspace
       //c.cursor.col--;
-      removeText(line, col-1, line, col);
+      removeText(line, col-1, line, col+complement);
     }
     else
     {
@@ -3044,11 +3095,11 @@
         }
         if (y < 0) {
           // FIXME: what shoud we do in this case?
-          removeText(line, 0, line, col);
+          removeText(line, 0, line, col+complement);
         }
       }
       else
-        removeText(line, col-1, line, col);
+        removeText(line, col-1, line, col+complement);
     }
   }
   else
Comment 22 Mathieu Jobin 2005-12-01 05:48:40 UTC
sorry there, there is something missing here.

when somebody is not use to the autobracket system, its annoying and we should handle that.

when autobracket is on, and the user type [] he will get []]
the duplicate should be automatically eliminated.

not sure how to do it properly though...
Comment 23 Dominik Haumann 2005-12-01 14:00:50 UTC
Then this is a dupe of bug #79200, isn` it?
Comment 24 Mathieu Jobin 2005-12-01 19:23:02 UTC
I already said something like that, but not really.
its related but different.

actually in bug #79200, there is two request.

first wish is an auto-indent improvement feature where If you press 
{ + ENTER

you get

{
   CURSOR HERE
}

with indent being preserved.

second wish is same thing that I stated here in comment #22
something that should be added to the patch found in this ticket.
Comment 25 Timo Bergmann 2006-01-17 13:25:04 UTC
I reported a bug, concerning the auto brackets. See bug report #120253

http://bugs.kde.org/show_bug.cgi?id=120253
Comment 26 Jan Jergus 2006-07-20 21:32:12 UTC
> - try to guess when a corresponding completion character is not wanted
> As it seems very few users use auto brackets (we all don't) it is hard to get feedback.

Well, I do and here's my feedback.

I haven't read the "guessing" code (and quite possibly I wouldn't understand it if I had anyway, I'm not that much of a programmer), but the current behaviour is certainly annoying, very annoying. I believe that with auto brackets enabled, Kate should always add the closing bracket, or at least there should be a special option in configuration that would enable/disable this "guessing".

I haven't yet come to a situation where I type opening bracket and don't want the closing bracket (please show me a good example), but the problem described in bug #120253 occurs to me very often when I write complex (or even quite simple) "if" statements like:
if (count($array) && ($a == $b))

To sum up, please either
1. resume the old auto brackets behaviour.
or
2. add an option to select old/new auto brackets behaviour. This option could be called "Try to guess when bracket is neccessary", so that no person in their right mind would enable it :)
Comment 27 Mathieu Jobin 2006-07-21 02:25:45 UTC
according to people's feedback, the problem seems to happen only with curly braces {}

I wonder, if maybe the guessing could be kept for other completion " ' [ but removed for {

that might help, not sure though. just a quick idea.

the old behaviour was very simple, did not include "' and was just duplicating the bracket. IMO it was way more annoying than the current behaviour. I also remember the guy who wrote the previous code saying it was just a quickly done, nothing to be proud of.

...

I just spent a few minutes reading the code and actually,... the "guessing" only apply to quotes and double quotes. none of the brackets.

I did a small test in kwrite

if (i want to ) {
	{soemthing {blabla{}}}
	{someting { test{}}}
	{something{test{fefe}}}
{test{}
{tedst{fekhe{}
}

the bug does not happened if I first indent the line. see the first three lines are fine, either I used spaces or not. the last two lines are buggy. the different is the indent. may I suspect this is a bug related with the indent code? or the indent code + auto bracket acting weird together.

i dont know but from what I can understand here, it ain't related.

actually I was kinda answering to bug #120253 here ;)

the "guessing" is contained all in this line.


+            if ( ( (ch == '\'' || ch == '"') &&
+                   (prevChar.isLetterOrNumber() || prevChar == ch) )
+              || nextChar.isLetterOrNumber()
+              || (nextChar == end_ch && prevChar != ch) ) 

very usefull when adding a ending quote at the end of a string for example.
and many others string/quote use. removing this would make the behaviour very annoying.
Comment 28 Mathieu Jobin 2006-07-21 02:29:06 UTC
in my kwrite test, the first three line inside the if contains a TAB in front.
I just realized it is not rendered by <your favorite html browser>.

Comment 29 Jan Jergus 2006-07-21 12:17:57 UTC
> the bug does not happened if I first indent the line.

You are right. The bug #120253 problem is not a behavior introduced by this "guessing", but it's really a bug.

It does not happen if you type
[TAB]if (something (

but it happens if you
- don't type the [TAB] first
- set "Use spaces to indent instead of tabs" in configuration
Comment 30 Dominik Haumann 2006-07-30 11:03:48 UTC
Ok, what about this solution:
- revert for { and [ for kde 3.5.5
- try to implement intelligent completion for { and [ in the indenter for kde4
Comment 31 Mathieu Jobin 2006-11-29 01:34:11 UTC
ok, there is a problem with my patch here for C++. 
I've been writing ruby almost exclusively since, and had no problem. but in languages like C++ that "overuses" parenthesis, it is sightly of a pain.

the problem happens when writing this

if (x == f())

it won't complete the inner () because of the following line in the patch

|| (nextChar == end_ch && prevChar != ch) ) 

and you will only get

if (x == f()

it's not too big of a deal maybe, I just type another ending parathesis in this specific case. 

but I wonder what smarter check could we do here. I guess the best would be to use a syntax parser to know if I am inside an incomplete block. but I'm afraid it might be slow. i don't know though.

another idea, would be to consider all blocks are closed. since the completion option is enabled. thus as a quick fix.

you could simply remove that check

|| (nextChar == end_ch && prevChar != ch)

and things should be better. it might even fix other problems mentioned earlier.

thanks
Comment 32 Dominik Haumann 2006-12-15 09:21:37 UTC
The most predictable way is to revert ;)
Comment 33 Wolfram R. Sieber 2007-01-29 00:55:21 UTC
First of all: Sorry, I didn't read the whole thread, only about 40% of it (up to "Mathieu Jobin 2005-08-13 18:59"). I hope, the things I've got to mention weren't considered yet.

Currently, I have to deal a lot with Mediawiki markup. Since the highlighting of [[... |xxx]] is broken (I filed that report somewhere else), I returned to no highlighting at all. I just mention that in case of highlighting influences autocompletion behaviour.

As indicated yet, Mediawiki code uses a lot of double characters, where normal text wouldn't use any such. So the otherwise pretty fine reduction of multiple ' characters results in a ''word' instead of the expected ''word''. Would be nice, if that could be fixed somehow.

Also, it would be nice if selected text plus a "[" key hit would leave the text selected (in case one's working on Mediawiki markup). So that one could get the second (necessary) bracket around the text -- without having to select it a second time. -- As a workaround, for now, I just cut the text, make a two spaces wide blank for it, hit the "[" twice and paste the word then. That's a bit fiddly, it works, but it's probably not a (good) long-term solution.

.
Lastly, I think, the current (KDE 3.5.5) solution of avoiding the closing brace if the next character is a letter could be improved. For example: Writing text, in the middle of a sentence, I notice an embraces addition before another word would improve the text. To move there I hit Ctrl-Left until the cursor arrives the first character after the blank. There, one normally would begin to type (and add another space after the then-typed word). Since it is an embraced addition, I want to do, the first character I hit, is "(". Completely unexpectedly, the closing brace doesn't appear. -- Hence, because I want to have it appear, now, I usually move the cursor to behind the (first) blank, hit the space key, move the cursor back once, and hit "(" then. Isn't that akward to do? (I'm not the only person who's inserting additions to its text in the middle of writing a sentence, am I?)
Comment 34 Mathieu Jobin 2007-04-24 16:57:13 UTC
once KDE4 and kate for KDE4 becomes stable, I'd like to put some time on this bug and get this functionnality working as 95% people would expect it.

please wake me up when the times comes.

and if you have algorythm design suggestions, please post them to this ticket please.

regards,

Mathieu

Comment 35 Dominik Haumann 2007-04-24 17:27:07 UTC
btw, I'd like to revert for KDE4.0, so that auto brackets simply adds a corresponding closing one, just like it was before. This 'feature' right now brings to much bugs and unpredictable behaviour.
Comment 36 Mathieu Jobin 2007-04-24 19:15:44 UTC
I think its gonna need to be per file type
the patch I wrote is quasi-perfect for ruby, but a pain for C/PHP
Comment 37 Renan Inácio 2007-10-09 03:10:20 UTC
I didn't read all the comments but something that makes auto bracketing painful:

If I type "(" + "X", I get (X), which is good.
Then if I type "(" + "X" + BACKSPACE + BACKSPACE, i get nothing, which is good too.
But if I type "(" + "X" + ")", I'll get (X)), which isn't good at all.

The same "memory" that the second case has (deleting opening bracket deletes the closing one), should be present when typing ")" just after the automatic one.
So to get (X)), I would type "(" + "X" + ")" + ")"
Comment 38 Jean-Philippe Fleury 2009-01-05 18:49:06 UTC
(In reply to comment #22)
> when autobracket is on, and the user type [] he will get []]
> the duplicate should be automatically eliminated.

I really agree with that. It would improve "Auto brackets" usability.
Comment 39 Frank Niethardt 2009-07-13 11:50:10 UTC
As told on #kate I have a IMHO simple idea to solve that problem of duplicated closing braces. I didn't look in the code, yet, so I don't know how much effort it is.

Idea is to have a stack with all closing parts that are inserted while using auto brackets.
- if you just continue to write and type the contents of the stack, it will be ignored/replaced instead of added to the text and poped from stack
- if you move the cursor right as pressing -> the brackets are poped from stack, too
- if you make any more complicated move, the stack is cleared completely

I think this will handle most of the cases where you are annoyed of "auto brackets"
Comment 40 Dominik Haumann 2009-12-05 14:17:17 UTC
SVN commit 1058975 by dhaumann:

revert non-deterministic insertion of brackets
- always insert ')' for '(', same for {["
- backspace only deletes one character

On the long run we need to find a different solution, depending on the current
filtype. Ideally, this needs to be scripted as well...

BUG:101213

 M  +21 -71    katedocument.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1058975
Comment 41 Dominik Haumann 2009-12-05 14:33:42 UTC
Ok, we've reverted this now to a deterministic behaviour.

This logic should not be hard coded anyway, because this heavily depends on which programming language you are writing. We hope to get this integrated into the scripting framework.

Please open a new bug report for further issues about this. This report is far too long already. Thanks.
Comment 42 Mathieu Jobin 2009-12-06 07:01:13 UTC
latest patch is crap
Comment 43 Dominik Haumann 2009-12-06 17:16:44 UTC
No it's not. For instance, bug #216383 is fixed now. Users are wondering why autobracket does sometimes not work. And that exactly is the reason for the revert.

Or let me frame it differently: The previous solution simply wasn't the right one.
I won't comment further on this here, open a new report, as said. And while we are at it: Please keep it constructive. We don't need unqualified comments. Thank you.
Comment 44 Dominik Haumann 2012-03-31 10:29:41 UTC
Git commit eba2b66fc16a0dce186a87b6376e4750635f43a9 by Dominik Haumann.
Committed on 31/03/2012 at 12:27.
Pushed by dhaumann into branch 'master'.

smarter auto-bracket feature

REVIEW: 104259

M  +33   -20   part/document/katedocument.cpp

http://commits.kde.org/kate/eba2b66fc16a0dce186a87b6376e4750635f43a9