Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Fix cursor position during auto-complete #1051

Merged

Conversation

lassoan
Copy link
Member

@lassoan lassoan commented Nov 2, 2022

When error messages were printed during auto-complete, the error messages got mixed up with the entered command.

Fixed by inserting text into the correct position (this->textCursor() was called twice in ctkConsolePrivate::printString() and so the movePosition() call had no effect); also changed the code to use this->textCursor() consistently to get the cursor (sometimes the equivalent QTextCursor c(this->document()) call was used)

Also fixed output printing: always print pending messages before displaying the prompt.

see Slicer/Slicer#6630

When error messages were printed during auto-complete, the error messages got mixed up with the entered command.

Fixed by inserting text into the correct position (this->textCursor()  was called twice in ctkConsolePrivate::printString() and so the movePosition() call had no effect); also changed the code to use this->textCursor() consistently to get the cursor (sometimes the equivalent `QTextCursor c(this->document())` call was used)

Also fixed output printing: always print pending messages before displaying the prompt.

see Slicer/Slicer#6630
@lassoan lassoan requested a review from jcfr November 2, 2022 01:44
@lassoan lassoan self-assigned this Nov 2, 2022
Copy link
Contributor

@jamesobutler jamesobutler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the same example I provided in Slicer/Slicer#6630, this is definitely better! I can actually hit enter to autocomplete the field and have it be added to the correct line.

python-console-autocomplete-updated.mp4

The errors printed during typing are the same as in Slicer 5.0.3. In Slicer 5.0.3, they were just only seen in the log file as viewed in Help->Report A Bug, but now with the improvements to the python console we see these errors in the python console. So from a user standpoint I might be questioning if I'm doing something wrong now that these errors are more prominently displayed. In this specific autocomplete typed case, is there something that I'm doing wrong or is it just executing code prematurely in a manner that is invalid?

@jcfr
Copy link
Member

jcfr commented Nov 2, 2022

In this specific autocomplete typed case, is there something that I'm doing wrong or is it just executing code prematurely in a manner that is invalid?

I suggest we revisit in a follow up PR, in the meantime, I will move forward with integration and update CTK.

@jcfr jcfr merged commit 2c89539 into commontk:master Nov 2, 2022
@lassoan lassoan deleted the fix-message-printing-during-autocomplete branch November 2, 2022 13:13
@lassoan
Copy link
Member Author

lassoan commented Nov 2, 2022

from a user standpoint I might be questioning if I'm doing something wrong now that these errors are more prominently displayed

The error messages are reported if a property getter of a Python-wrapped Qt object reports an error (see for example a freshly created slicer.qMRMLTableView(), without setting a table node in it). I've created an issue to track this and added more information and potential solutions - see #1054

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants