gh-60436: fix curses textbox backspace/del

[aidanmelen]

fixes issue: #60436

update curses textbox to additionally handle backspace using the curses.ascii.DEL key press.

My current workaround is to pass a custom validate func that handles the additional key press for backspace.

• Issue: curses.textpad.Textbox backspace support #60436

[bedevere-bot]

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

[ghost]

All commit authors signed the Contributor License Agreement.

[aidanmelen]

I tested locally by running python Lib/curses/textpad.py and I was able to delete characters with the the "delete" key on mac.

[mblahay]

@aidanmelen Are there any tests you can add? I know this is a difficult question/request. I am discussing with the team here at pycon. Any ideas you may have would be helpful.

[aidanmelen]

I didn't see any existing tests for curses.textpad so I didn't really consider it.

Enjoy PyCon!

[aidanmelen]

Okay, I didn't look very hard. There are two curses related files:

• https://github.com/aidanmelen/cpython/blob/main/Lib/test/test_curses.py
• https://github.com/aidanmelen/cpython/blob/main/Lib/test/curses_tests.py

where curses_tests.py is just an example.

[mblahay]

Yep, I found those files as well, but you will need to add to one of them.

[mblahay]

I was able to modify the code from testpad.py into a proof of concept showing we can test that the actions performed on the textbox produce the desired output. Take a look at this and see if it might be useful.

import curses
import curses.ascii
from curses.textpad import *

def test_editbox(stdscr):
	ncols, nlines = 9, 4
	uly, ulx = 5, 20
	stdscr.addstr(uly-2, ulx, "Use Ctrl-G to end editing.")
	win = curses.newwin(nlines, ncols, uly, ulx)
	rectangle(stdscr, uly-1, ulx-1, uly + nlines, ulx + ncols)
	stdscr.refresh()
	textbox = Textbox(win)
	textbox.do_command('A')
	textbox.do_command('B')
	textbox.do_command('C')
	textbox.do_command('D')
	textbox.do_command('E')
	textbox.do_command('F')
	textbox.do_command('G')
	textbox.do_command('H')
	textbox.do_command('I')
	textbox.do_command('J')
	textbox.do_command('J')
	textbox.do_command(curses.ascii.BS)
	textbox.do_command('K')
	
	#textbox_contents = textbox.edit()
	textbox_contents = textbox.gather()
	
	return textbox_contents

str = curses.wrapper(test_editbox)
print('Contents of text box:', repr(str))

Just throw it into a file and execute. In the commands above, I "accidentally" add an extra J, then an backspace is submitted. The final output shows the extra J has been removed. I know the test isn't perfect since it isn't emulating an exact keypress on a mac keyboard, but it is something.

[mblahay]

Just realized I should have used curses.ascii.DEL in the example above. Actually, probably should have used all that can remove a character. Anyways, you get the idea....

[aidanmelen]

@mblahay I wrote a basic unittest that uses dependency injection to mock the curses.window in the TextBox class and assert that various curses methods are called when specific ch values are passed. I made sure to test that self.win.delch() is called when all of curses.ascii.BS, curses.KEY_BACKSPACE and curses.ascii.DEL.

I don't like having the import statements in the class but I there seemed to be some logic that skipped test when curses was not available and I didn't want to break that logic with imports at the top.

[mblahay]

I have looked at the code that uses the mock, and it does seem to verify the functionality of Textbox, while not actually needing to deal with an ncurses window. @ambv, @ned-deily what do you think?

[mblahay]

As we are at it, why is the Azure Pipelines PR test failing? I have dug into it to a point where it is saying the issue is whitespace in the test file, but nothing appears to be wrong with the whitespace.

[aidanmelen]

the test_curses.py is passing now. I think this is ready for review.

[mblahay]

Changes look good. Good use of mocking for a difficult test situation.

[mblahay]

@ambv This is ready to move forward.

[ambv]

I'm unable to confirm this fixes the issue as demonstrated in the StackOverflow post:

import curses
import curses.textpad

def get_input(window):
    curses.curs_set(1)
    curses.set_escdelay(1)
    window.addstr(0, 0, "Enter some text:")
    input_box = curses.textpad.Textbox(curses.newwin(1, 40, 1, 0), insert_mode=True)
    
    def validate(ch):

            # exit input with the escape key
            escape = 27
            if ch == escape:
                ch = curses.ascii.BEL # Control-G
            
            # delete the character to the left of the cursor
            elif ch in (curses.KEY_BACKSPACE, curses.ascii.DEL):
                ch = curses.KEY_BACKSPACE
            
            # exit input to resize windows
            elif ch == curses.KEY_RESIZE:
                ch = curses.ascii.BEL # Control-G

            return ch
    
    input_box.edit(validate)
    curses.curs_set(0)
    curses.set_escdelay(1000)
    return input_box.gather().strip()


def main(window):
    curses.noecho()
    curses.cbreak()
    window.keypad(True)
    curses.curs_set(0)

    input_text = get_input(window)

    window.addstr(2, 0, f"You entered: {input_text}")
    window.refresh()
    window.getch()

if __name__ == "__main__":
    curses.wrapper(main)

This is still not handling the Delete key (Fn+Backspace) on an M1 Mac, macOS 12.6.5. I tested with iTerm2 and the built-in Terminal.app, with Fish and Bash. No combination works.

[ambv]

Running ./python.exe -m curses.textpad similarly demonstrates that the issue persists.

[ambv]

I retract my previous comment about the change not fixing the issue. There's some confusion about what "DEL" and "delete" means. To me it means "remove character on the right of the cursor".

In this PR we are using curses.ascii.DEL to fix the backspace (i.e. "remove character on the left of the cursor") behavior. This indeed works on my Mac.

[aidanmelen]

@ambv yes, that is also my understanding! Thanks for providing your context.

[mblahay]

@aidanmelen Please make the changes @amb is suggesting. Once those are done, this can proceed to merge.

[aidanmelen]

thanks for the suggestions @ambv. @mblahay ready for review again.

[hugovk]

This has been fixed in the 3.12 (and now main/3.13) branches.

Should it also be backported to 3.11?

If so, let's also include the test fix from #106196.