wxWidgets / Phoenix

wxPython's Project Phoenix. A new implementation of wxPython, better, stronger, faster than he was before.
http://wxpython.org/
2.33k stars 515 forks source link

PositionToXY() / XYToPosition() fail at end of text #2408

Open RF3 opened 1 year ago

RF3 commented 1 year ago

Operating system: macOS 12.6.6 wxPython version & source: 4.2.1 (source: pip3) Python version & source: Python 3.10.11 (inside virtual environment, installed with homebrew)

Description of the problem:

I ran into an issue while using PositionToXY() / XYToPosition() with StyledTextCtrl. PositionToXY() fails to return valid column and line numbers when the caret is located after the very last character of the text. Instead, the first value of the returned tuple is False indicating that the given position is invalid. But it is valid, which can be verified by calling GetColumn() and GetCurrentLine().

When converting column and line into a position with XYToPosition(), a similar error happens. The return value is -1, indicating the given column or line is invalid. Yet again, when the caret position is right of the last character, it is a valid position that can be used with GotoPos(), for example.

Code Example (click to expand) ```python #!/usr/bin/env python3 # -*- coding: utf-8 -*- import sys import wx import wx.stc as stc class MyFrame(wx.Frame): def __init__(self, area): # MyFrame overrides wx.Frame, so we have to call it using super: super(MyFrame, self).__init__(None, pos=(area.x, area.y), size=(800, 600)) self.TextObj = stc.StyledTextCtrl(self) self.TextObj.SetWindowStyle(self.TextObj.GetWindowStyle() | wx.DOUBLE_BORDER) layout = wx.BoxSizer(wx.HORIZONTAL) layout.Add(self.TextObj, proportion=1, border=0, flag=wx.ALL|wx.EXPAND) self.SetSizer(layout) self.step = 1 def run(expression, withresult=False, moretext=""): eval_expression = "self.TextObj." + expression if withresult: result = eval(eval_expression) print("Step #{}: {} = {} {}".format(self.step, expression, result, moretext)) else: print("Step #{}: Calling {}".format(self.step, expression)) eval(eval_expression) self.getpos() self.step = self.step + 1 print("Creating content: 0123456789 (no newline at the end; 10 characters)") run("SetValue('0123456789')") run("GotoPos(9)") run("GetCharAt(9)", True, "(ASCII code)") run("GotoPos(10)") run("GetCharAt(10)", True, "(ASCII code)") run("XYToPosition(9, 0)", True) run("GetCharAt(9)", True, "(ASCII code)") run("XYToPosition(10, 0)", True) run("GetCharAt(10)", True, "(ASCII code)") self.Destroy() def getpos(self): getcurrentpos = self.TextObj.GetCurrentPos() success, column, line = self.TextObj.PositionToXY(getcurrentpos) getcolumn = self.TextObj.GetColumn(getcurrentpos) getcurrentline = self.TextObj.GetCurrentLine() print(" => getcurrentpos = {}, success = {}, column = {}, line = {}, getcolumn = {}, getcurrentline = {}".format(getcurrentpos, success, column, line, getcolumn, getcurrentline)) class MyApp(wx.App): def OnInit(self): frame = MyFrame(area=wx.Display().GetClientArea()) self.SetTopWindow(frame) return True def main(): app = MyApp(False) app.MainLoop() if __name__ == "__main__": main() ```

The example code creates a StyledTextCtrl with a text control value consisting of 10 characters (just the digits 0 to 9) without newline at the end. Therefore, a valid column number is 0 to 10 because the caret can be right of the last character as position 9.

After each call, the program returns the value of the GetCurrentPos() method plus the three values of PositionToXY(), starting with the boolean value and followed by column and line, plus the returned values of GetColumn() and GetCurrentLine(). This helps to understand the issue and demonstrates the inconsistant behaviour.

At step #2, the caret is positioned with GotoPos(9) over the last character '9' and when GetChar() is called in step #3, it returns ASCII code 57 which is indeed the '9'. In step #4, the caret is positioned after the last character with GotoPos(10) and GetCurrentPos() equals 10 and therefore confirms the caret position has changed from 9 to 10, but PositionToXY() returns False (column = 140701938442352 and line = 4428290009) while GetColumn() returns 10. So, PositionToXY() tells us it is not possible to place the caret in column 10 but GetCurrentPos() and GetColumn() confirm that GotoPos(10) worked as expected. GetChar() returns ASCII code 0 at this position so position 10 is not an illegal index.

In step #6, we're now doing the opposite conversion with XYToPosition(). XYToPosition(9, 0) for column 9 returns position 9. That's correct. But in step #8, XYToPosition(10, 0) returns -1 while GetColumn() returns 10 for position 10.

Output (click to expand) ```code Creating content: 0123456789 (no newline at the end; 10 characters) Step #1: Calling SetValue('0123456789') => getcurrentpos = 0, success = True, column = 0, line = 0, getcolumn = 0, getcurrentline = 0 Step #2: Calling GotoPos(9) => getcurrentpos = 9, success = True, column = 9, line = 0, getcolumn = 9, getcurrentline = 0 Step #3: GetCharAt(9) = 57 (ASCII code) Step #4: Calling GotoPos(10) => getcurrentpos = 10, success = False, column = 140701938442352, line = 4428290009, getcolumn = 10, getcurrentline = 0 Step #5: GetCharAt(10) = 0 (ASCII code) Step #6: XYToPosition(9, 0) = 9 Step #7: GetCharAt(9) = 57 (ASCII code) Step #8: XYToPosition(10, 0) = -1 Step #9: GetCharAt(10) = 0 (ASCII code) ```

Conclusion:

It's obvious that PositionToXY() and XYToPosition() don't accept a position or column after the last character, but methods like GetCurrentPos() or GetColumn() return valid values confirming that this caret position set with GotoPos() is in fact valid.

BTW, I don't understand why PositionToXY() returns these weird values 140701938442352 and 4428290009 for an invalid position. A column value of -1 and a line value of -1 would make much more sense. The first value of the tuple wouldn't be needed anymore.

Edit 06/08/2023:

I just installed wxPython 4.2.1, but nothing has changed.

swt2c commented 1 year ago

Thank you for the very excellent and thorough bug report. Unfortunately, this seems to be an upstream issue in wxWidgets introduced by https://github.com/wxWidgets/wxWidgets/commit/6e556d4a71d3c9cc5982902d04d15f000e128d20.

swt2c commented 1 year ago

Upstream issue has a patch that seems to resolve the issue: https://github.com/wxWidgets/wxWidgets/issues/18430

RF3 commented 1 year ago

I think this patch is too complicated and assumes that the case of x == LineLength(y) is not allowed except for the last line. If we have, for example, 10 characters in a line (not necessarily the last one), then column 10 is a valid position for the caret and therefore x == 10 shouldn't result in returning False. In my opinion, it seems that changing

if ( x >= LineLength(y)

to

if ( x > LineLength(y)

and similar

if ( lx >= LineLength(l) )

to

if ( lx > LineLength(l) )

would already fix this issue.