Hi, Referring to *bug Auto-close parenthesis - ID: 2957958 ( https://sourceforge.net/tracker/?func=detail&aid=2957958&group_id=15...) I successfully reproduced the issue and was able to debug it. The problem is in file editor.c function auto_close_char Line:1397
In the switch-case block the ifs have a compulsory condition which is end_pos == -1 , as far as I understand end_pos is the current position of the brace ( caret ) returned by SSM(sci, SCI_BRACEMATCH, pos, 0) and when the second opening brace is typed the end_pos becomes >0 so the code never enters the if block. Just by removing the condition the problem is fixed.I was unable to get why that specific condition was added, can anybody please help me here?
Also if there is auto completion of braces when a '(' is typed why there is no auto removal of ')' when a brace is deleted?
I am sorry if the mail looks pretty unprofessional but I have just begun to learn geany code base. Can anybody please assign the bug to me? I dont know how to do it in SF I think only the Project admins can do it. Any help is appreciated.
Thank you shankhs *
On 2 November 2010 05:16, shan chak shankholove@gmail.com wrote:
Hi, Referring to bug Auto-close parenthesis - ID: 2957958 ( https://sourceforge.net/tracker/?func=detail&aid=2957958&group_id=15... ) I successfully reproduced the issue and was able to debug it.
Great, thank you.
The problem is in file editor.c function auto_close_char Line:1397
In the switch-case block the ifs have a compulsory condition which is end_pos == -1 , as far as I understand end_pos is the current position of the brace ( caret ) returned by SSM(sci, SCI_BRACEMATCH, pos, 0) and when the second opening brace is typed the end_pos becomes >0 so the code never enters the if block. Just by removing the condition the problem is fixed.I was unable to get why that specific condition was added, can anybody please help me here?
I haven't analyzed it carefully, but I think what it is trying to do is to only add a new close bracket only if one is needed. If sci_find_matching_brace finds a match then it won't add one, whereas if there is no matching close bracket then one will be added.
Of course the use case shown in the bug shows this is flawed in some cases, but in other cases its useful eg if the end == -1 wasn't there
a = ( b + 1 ? c : d ) + 1 oops I need a bracket around the b+1
type the (
a = ( () b + 1 ? c : d ) + 1 ... !@#$%^&*()
So you can't win, no simple algorithm is right all the time. I guess the autoclose users need to decide which is more useful (I don't use it so I don't care)
Also if there is auto completion of braces when a '(' is typed why there is no auto removal of ')' when a brace is deleted?
No one wrote the code, patches are welcome, but beware of similar issues to above. Actually I think this patch may not be welcome since silently deleting characters is *bad*.
I am sorry if the mail looks pretty unprofessional but I have just begun to learn geany code base.
No problem.
Cheers Lex
Can anybody please assign the bug to me? I dont know how to do it in SF I think only the Project admins can do it. Any help is appreciated.
Thank you shankhs
Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Hi, Thanks Lex,
The problem is in file editor.c function auto_close_char Line:1397
In the switch-case block the ifs have a compulsory condition which is end_pos == -1 , as far as I understand end_pos is the current position of the brace ( caret ) returned by SSM(sci, SCI_BRACEMATCH, pos, 0) and when the second opening brace is typed the end_pos becomes >0 so the code
never
enters the if block. Just by removing the condition the problem is
fixed.I
was unable to get why that specific condition was added, can anybody
please
help me here?
I haven't analyzed it carefully, but I think what it is trying to do is to only add a new close bracket only if one is needed. If sci_find_matching_brace finds a match then it won't add one, whereas if there is no matching close bracket then one will be added.
Of course the use case shown in the bug shows this is flawed in some cases, but in other cases its useful eg if the end == -1 wasn't there
a = ( b + 1 ? c : d ) + 1 oops I need a bracket around the b+1
type the (
a = ( () b + 1 ? c : d ) + 1 ... !@#$%^&*()
So you can't win, no simple algorithm is right all the time. I guess the autoclose users need to decide which is more useful (I don't use it so I don't care)
I missed this case. Surely its pretty annoying. Now if we see how eclipse implement this , it will never autocomplete the braces for cases like these: a = ( b + 1 ? c : d ) + 1 Now if we type the bracket around b+1 then a = ( ( b + 1 ? c : d ) + 1 and let the user decide how to close it. The algorithm is complex taking into consideration many corner cases.
Also if there is auto completion of braces when a '(' is typed why there
is
no auto removal of ')' when a brace is deleted?
No one wrote the code, patches are welcome, but beware of similar issues to above. Actually I think this patch may not be welcome since silently deleting characters is *bad*.
Yeah its bad, but take the case:
A user inputs '(' and ')' is automatically added, now what if user deletes the '(' , he/she also has to delete the ')'. I think in such cases its not a big risk to delete the ')'. Like the above case if there is any character in between '(' and ')' let the user decide.
I am sorry if the mail looks pretty unprofessional but I have just begun
to
learn geany code base.
No problem.
Eagerly waiting for your comments.
Thank you shankhs
On 3 November 2010 05:19, shan chak shankholove@gmail.com wrote:
Hi, Thanks Lex,
The problem is in file editor.c function auto_close_char Line:1397
In the switch-case block the ifs have a compulsory condition which is end_pos == -1 , as far as I understand end_pos is the current position of the brace ( caret ) returned by SSM(sci, SCI_BRACEMATCH, pos, 0) and when the second opening brace is typed the end_pos becomes >0 so the code never enters the if block. Just by removing the condition the problem is fixed.I was unable to get why that specific condition was added, can anybody please help me here?
I haven't analyzed it carefully, but I think what it is trying to do is to only add a new close bracket only if one is needed. If sci_find_matching_brace finds a match then it won't add one, whereas if there is no matching close bracket then one will be added.
Of course the use case shown in the bug shows this is flawed in some cases, but in other cases its useful eg if the end == -1 wasn't there
a = ( b + 1 ? c : d ) + 1 oops I need a bracket around the b+1
type the (
a = ( () b + 1 ? c : d ) + 1 ... !@#$%^&*()
So you can't win, no simple algorithm is right all the time. I guess the autoclose users need to decide which is more useful (I don't use it so I don't care)
I missed this case. Surely its pretty annoying. Now if we see how eclipse implement this , it will never autocomplete the braces for cases like these: a = ( b + 1 ? c : d ) + 1 Now if we type the bracket around b+1 then a = ( ( b + 1 ? c : d ) + 1 and let the user decide how to close it. The algorithm is complex taking into consideration many corner cases.
Since you need to know the algorithm to implement it, perhaps you should write it out here for review. And remember this code is run for all filetypes, you might need to think about impacts on non-C filetypes.
Also if there is auto completion of braces when a '(' is typed why there is no auto removal of ')' when a brace is deleted?
No one wrote the code, patches are welcome, but beware of similar issues to above. Actually I think this patch may not be welcome since silently deleting characters is *bad*.
Yeah its bad, but take the case: A user inputs '(' and ')' is automatically added, now what if user deletes the '(' , he/she also has to delete the ')'. I think in such cases its not a big risk to delete the ')'. Like the above case if there is any character in between '(' and ')' let the user decide.
Probably ok if ) is immediately after (, the user is sure to see it go. But even so this should be off by default and turned on by a preference.
Cheers Lex
I am sorry if the mail looks pretty unprofessional but I have just begun to learn geany code base.
No problem.
Eagerly waiting for your comments.
Thank you shankhs
Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Hi,
Since you need to know the algorithm to implement it, perhaps you
should write it out here for review. And remember this code is run for all filetypes, you might need to think about impacts on non-C filetypes.
Thanks Lex, I was primarily thinking only C/C++ and Java. Let me develop the algo first. Need to keep in mind the various filetypes and uses of parenthesis in all of them.
Probably ok if ) is immediately after (, the user is sure to see it go. But even so this should be off by default and turned on by a preference.
Let me create a patch and test.
Cheers Lex
Thank you
shankhs
Hi,
Since you need to know the algorithm to implement it, perhaps you should write it out here for review. And remember this code is run for all filetypes, you might need to think about impacts on non-C filetypes.
You were correct , my bad :( I tried a lot to create something generic but
there was always some filetypes which would create an annoying exception and thus at the end the code became so filetype specific that at the end I had to give up the idea.
Probably ok if ) is immediately after (, the user is sure to see it go. But even so this should be off by default and turned on by a preference.
I found the case while testing (() this is perfectly legal. Now if we delete the 2nd open brace the closing brace should not be deleted, so the deletion is only applicable when the ) is just added . I think selective removal will create a lot of confusion to the end users so removed the code for this also :(
My first attempt went in vain :( . Searching for other bugs to debug.
Thanks shankhs