Brackets completion - #934

Codecov Report

❌ Patch coverage is 94.70899% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.69%. Comparing base (18f6901) to head (2c683fc).
⚠️ Report is 357 commits behind head on main.

@@            Coverage Diff             @@
##             main     #934      +/-   ##
==========================================
+ Coverage   67.98%   68.69%   +0.71%     
==========================================
  Files          61       62       +1     
  Lines        9174     9437     +263     
==========================================
+ Hits         6237     6483     +246     
- Misses       2937     2954      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I just played with this a bit. Since this would be an optional feature, these things wouldn't be required from my perspective (maybe none of them are worth the complexity!) but might be nice.

Overwriting closing characters

This already works for ) and ], but I'm missing it for strings too.

>>> s = |
>>> s = '|' # after pressing '
>>> s = 'a|'  # after pressing a
>>> s = 'a|'''  # after pressing '

To avoid the case above, a closing character could overwrite an existing one in some cases. This might be tricky to implement but it is behavior that makes autoclosing pairs in IDEs manageable for me, because I can still type what I normally would. Always overwriting any closing characters, ignoring whether they were automatically added, sounds easier.

Don't autoclose when cursor is followed by non-whitespace

I often forget that a socket.socket's connect method takes a single argument, a tuple:

>>> s = s.connect('localhost', 8080|)
>>> s = s.connect('localhost', |8080)  # back word
>>> s = s.connect(|'localhost', 8080)  # back a bunch more
>>> s = s.connect((|)'localhost', 8080)  # after pressing (

To avoid this, autoclosing could take place only when the the cursor is at the end of a line? I notice VSCode uses a fancier heuristic than this, but it seems based on what is to the right of the cursor. If it's a space or nothing, it seems to auto-close.

Remove autoclose when open is deleted (by method other than backspace)

This already works for backspace, but not some other ways of clearing the line:

>>> x = |
>>> x = (|)  # after pressing (
>>> x = (1|)  # after pressing 1
>>> x = |)  # after pressing ctrl-w

>>> x = |
>>> x = (|)  # after pressing (
>>> |)  # after pressing ctrl-u

VSCode has the same behavior as this PR, so I can hardly complain — but there are series of keystrokes I make similar to backspace that delete the opening character without deleting the matching close character.

Matching history entries

A closing paren causes history entries to no longer match.

>>> def foo(a, b, c):
...     pass
>>>

>>> d|   # after pressing d, fish-style history shows the previously typed line in grey
>>> def foo(|)  # after pressing (, the `def foo(a, b, c):` line from history is no longer suggested

Not a big issue, but the the closing paren can cause lines from history to no longer match. Maybe it could be removed when searching through history.

@thomasballinger i will read through this and get back to you

Overwriting closing characters

This already works for ) and ], but I'm missing it for strings too.

>>> s = |
>>> s = '|' # after pressing '
>>> s = 'a|'  # after pressing a
>>> s = 'a|'''  # after pressing '

To avoid the case above, a closing character could overwrite an existing one in some cases. This might be tricky to implement but it is behavior that makes autoclosing pairs in IDEs manageable for me, because I can still type what I normally would. Always overwriting any closing characters, ignoring whether they were automatically added, sounds easier.

• this issue is present because this part of a "switch" in repl.py on line 793:

...
elif e in CHARACTER_PAIR_MAP.keys():
    self.insert_char_pair_start(e)
elif e in CHARACTER_PAIR_MAP.values():
    self.insert_char_pair_end(e)
...

The problem is that opening and closing characters are the same and are difficult to distinguish. i will look into it and come up with a solution. BTW, tabbing still works even with quotes.

Don't autoclose when cursor is followed by non-whitespace

• this shouldnt be a problem to implement

Remove autoclose when open is deleted (by method other than backspace)

• this one is difficult to decide. i am also used to VS code behaviour and it does this only on backspace action. not even DEL key removes the closing bracket. i think we should leave it as is IMO. what do you think?

Matching history entries

• shouldnt be a problem either. i can print the closing paren to the line but omit it afterwards

These changes shouldnt take a lot of time.

BTW, tabbing still works even with quotes.

great, that's good

this one is difficult to decide. i am also used to VS code behaviour and it does this only on backspace action. not even DEL key removes the closing bracket. i think we should leave it as is IMO. what do you think?

Sounds fine to me, I'm not sure how I'd go about this either.

shouldnt be a problem either. i can print the closing paren to the line but omit it afterwards

Sounds good, but if it's very complicated feel free to skip this one — it's something that I noticed, but I'm not sure it's worth fixing.

@thomasballinger

Don't autoclose when cursor is followed by non-whitespace

I often forget that a socket.socket's connect method takes a single argument, a tuple:

>>> s = s.connect('localhost', 8080|)
>>> s = s.connect('localhost', |8080)  # back word
>>> s = s.connect(|'localhost', 8080)  # back a bunch more
>>> s = s.connect((|)'localhost', 8080)  # after pressing (

To avoid this, autoclosing could take place only when the the cursor is at the end of a line? I notice VSCode uses a fancier heuristic than this, but it seems based on what is to the right of the cursor. If it's a space or nothing, it seems to auto-close.

i just realized, when testing the solution for this problem, that it breaks another good behaviour. if you write multiple nested open parens (|) you expect that you will be moved inside them ((|)). however if we limit the autoclosing by the following character (in this case ), which is not end of line or space, it effectively destroys this feature. it becomes ((|). we could maybe take all closing brackets, excluding quotes, as allowed chars (right of the cursor) when autocompleting. thoughts?

Hm, maybe the behavior I think I want is too complex to be worth implementing. What I see in VSCode (which I think does what I want, but I'm not even sure about that — it does work for this one socket case) is:

   ( ( ( ) ) )   # ignore the spaces
  | | |          # adding a left paren ( here does not autoinsert a right paren )
        | | | |  # adding a left paren ( here does autoinsert a right paren )

@thomasballinger thats exactly what i meant by previous comment! (pseudocode):

if next_char in [ ')', ']', '}' , ' ']: # or end of line
    complete_bracket()
else:
    pass

this way, the case with the socket works. basically it will autocomplete very similarly to VS Code
EDIT: see latest commit

Matching history entries

A closing paren causes history entries to no longer match.

>>> def foo(a, b, c):
...     pass
>>>

>>> d|   # after pressing d, fish-style history shows the previously typed line in grey
>>> def foo(|)  # after pressing (, the `def foo(a, b, c):` line from history is no longer suggested

Not a big issue, but the the closing paren can cause lines from history to no longer match. Maybe it could be removed when searching through history.

i looked into the missing history hints, and basically the problem seems to be that the add_normal_character method called cursor_offset setter, which called _set_cursor_offset method with update_completion default param set to True this was triggering the unwanted update. should be okay now. i also moved the bracket completion to the end of the method on_tab, because it was triggering before the search completion and did override it. try it out, should be better now.

@thomasballinger
i can try to write some tests for batch of different scenarios. do you have some guidelines concerning that? should i create a new file or put it elsewhere? thanks!

@thomasballinger i can try to write some tests for batch of different scenarios. do you have some guidelines concerning that? should i create a new file or put it elsewhere? thanks!

Whatever is convenient for you! A new file sounds fine to me. If not a new file, bpython/test/test_curtsies_repl.py might be the right file.

@thomasballinger i wrote test for scenarios we talked about, including quotes and all brackets with TAB, BACKSPACE and others. i just cant figure out how to write a test which would include the search history. any advice, pointer? thanks!

I think you can check the value of self.rl_history.saved_line? If it's not populated, may need to call self.rl_history.enter('def foo(a, b, c):') to populate it.

Oh if you're checking for the grayed out text completion you might need to set self.config.curtsies_right_arrow_completion

thank you! any other ideas to add to this feature?

Seems good to me! I'm looking forward to it. option-r reverse history search is the only thing that I think is left to change.

This looks good to me! I'll wait for CI to run then merge it.

This looks good to me! I'll wait for CI to run then merge it.

great! hopefuly it will serve the users good

hey I am the one who opened this enhancement thanks for merging this
but I edited my config but I doesn't work,Is it not available for the version installed from pip?
Can you guide me how to get the feauture you implemented?

edit:I am new to python and english

@yogeshdcool I didn't merge this feature in in time for the release that came out a few days ago, so you'll either need to wait for the next release or install bpython from source. Here's how to do that with pip:

python -m pip install git+https://github.com/bpython/bpython.git@main

Thanks for helping me out