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

ENH: Improve robustness of Python code running in the console #1046

Merged
merged 2 commits into from
Oct 24, 2022

Conversation

lassoan
Copy link
Member

@lassoan lassoan commented Oct 22, 2022

When a complete string of command is executed (either copy-pasting or running from file) then we no longer split the string to lines and execute line by line because that often leads to errors. For example, parsing of this Python code line by line would fail due to the empty line in the function definition:

def test1():
    print("one")

    print("two")
    print("three")

def test2():
    print("ok")

Also make the console remember the path of the last run Python file to make it easier to run it again.

Before this fix (failure while trying to specify test1() function):

image

After the fix (everything works as expected):

image

When a complete string of command is executed (either copy-pasting or running from file) then we no longer split the string to lines and execute line by line because that often leads to errors. For example, parsing of this Python code line by line would fail due to the empty line in the function definition:

```
def test1():
    print("one")

    print("two")
    print("three")

def test2():
    print("ok")
```

Also make the console remember the path of the last run Python file to make it easier to run it again.
@lassoan lassoan requested a review from jcfr October 22, 2022 02:31
@lassoan lassoan self-assigned this Oct 22, 2022
@jamesobutler
Copy link
Contributor

Thanks! What you have described is something I’ve dealt with over the years when copy/pasting some functions.

@jcfr
Copy link
Member

jcfr commented Oct 22, 2022

Thanks for working on this 🙏

It is a great improvement that will greatly improve productivity.

Since I am reviewing on my phone, I couldn't test. That said, I spotted few typos in the comments (more more, muiltiline)

@jcfr
Copy link
Member

jcfr commented Oct 22, 2022

The improvements to the runfile function make sense, especially the saving and restoring of the current command.

If systematically displaying the entire file in the console turns out to be a problem, it could be revisited to only display the first N lines of (or first N and last N).

@lassoan
Copy link
Member Author

lassoan commented Oct 22, 2022

Thanks for the feedback. I've fixed the typos and changed the behavior when running a file to print just a message (Execute: file/to/script.py) instead of the file content.

@lassoan
Copy link
Member Author

lassoan commented Oct 24, 2022

I've addressed the review comments. I'll merge this now to give a chance for everyone to try this.

Libs/Widgets/ctkConsole.cpp Outdated Show resolved Hide resolved
Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
@lassoan
Copy link
Member Author

lassoan commented Oct 24, 2022

Thanks @jcfr, I've fixed one more more typo but apparently there were two occurrences.

@lassoan lassoan merged commit f9ae6d6 into commontk:master Oct 24, 2022
@lassoan lassoan deleted the console-exec-improvements branch October 24, 2022 17:00
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