WIP: add python console #144
Conversation
tigercosmos
force-pushed
the
python-console
branch
from
7043147
to
ec8a571
Compare
cpp/modmesh/view/RMainWindow.cpp
Outdated
| m_pyconsole = console; | ||
| m_pyconsole->setAllowedAreas(Qt::LeftDockWidgetArea | Qt::RightDockWidgetArea); | ||
|
|
||
| m_pytext = new RPythonText(QString("Python"), this, Qt::WindowFlags(), console); |
There was a problem hiding this comment.
share the console to RPythonText, since we need to update the output
|
|
||
| void RPythonConsole::setConsole(std::string const & stdout_str, std::string const & stderr_str) | ||
| { | ||
| std::stringstream ss; |
There was a problem hiding this comment.
could be a temporary format
a more complete version can be in future work, such as red color for error.
There was a problem hiding this comment.
Use modmesh::Formatter for string formatting.
| @@ -86,14 +95,19 @@ def get_appenv(name=None): | |||
| get_appenv(name='master') | |||
|
|
|||
|
|
|||
| def run_code(code): | |||
| def run_code(code, redirectStdOutFile=None, redirectStdErrFile=None): | |||
There was a problem hiding this comment.
a possible improvement could be recording the history of the outputs.
There was a problem hiding this comment.
This is not a right way to redirect buffered I/O in either Python or C++.
Add a dock widget at the bottom of the main window that a horizontally split text edit areas. The upper and larger one is for displaying the history of the input Python commands. The lower and narrow one is to input a Python command for execution. The command line supports history navigation through up and down arrow keys on the keyboard, and uses enter for sending the command for execution. It does not yet support multi-line Python code. Improvements that are needed in the future: * The output from Python goes to the process stdout and stderr that are not redirected to the Qt widget. It should be in the future. * The Python code dock widget on the right-hand side of the main window will be kept for a while before all the apps are migrated to use the new Python command console.
cpp/modmesh/view/RMainWindow.hpp
Outdated
| R3DWidget * viewer() { return m_viewer; } | ||
|
|
||
| private: | ||
|
|
||
| void setUp(); | ||
|
|
||
| RPythonText * m_pytext = nullptr; | ||
| std::shared_ptr<RPythonConsole> m_pyconsole = nullptr; |
There was a problem hiding this comment.
A very good reason is needed to justify the use of shared pointer with Qt objects, which has its own ownership system. Shared pointer does not work well with Qt pointers. I do not see why you need the shared pointer here.
There was a problem hiding this comment.
I use shared_ptr because the pointer is shared. I think a raw pointer is also fine.
There was a problem hiding this comment.
You should use signal/slot for accessing other Qt objects to make the event loop and ownership manager happy.
There was a problem hiding this comment.
could you give some references?
There was a problem hiding this comment.
Googling the keywords in the sentence should be sufficient.
| set_target_properties( | ||
| viewer | ||
| PROPERTIES | ||
| LINK_FLAGS_DEBUG "/SUBSYSTEM:CONSOLE" # open the console for debug mode |
There was a problem hiding this comment.
If it opens an additional debug console in release build, it needs to be changed.
There was a problem hiding this comment.
in release mode, there is no cmd prompt.
There was a problem hiding this comment.
To make it apparent, I recommend to only add the switch when it's not in release mode.
|
|
||
| void RPythonConsole::setConsole(std::string const & stdout_str, std::string const & stderr_str) | ||
| { | ||
| std::stringstream ss; |
There was a problem hiding this comment.
Use modmesh::Formatter for string formatting.
cpp/modmesh/view/RPythonText.cpp
Outdated
| @@ -65,16 +69,50 @@ void RPythonText::runCode() | |||
| { | |||
| namespace py = pybind11; | |||
|
|
|||
| const std::string redirect_stdout_file_path = "stdout.txt"; | |||
There was a problem hiding this comment.
We may not redirect buffered I/O to files unconditionally like this.
There was a problem hiding this comment.
what is a proper way?
There was a problem hiding this comment.
If by default it's redirected, there must be a runtime-configurable way to change it back. If it's undirected by default, a runtime-configurable way needs to be provided to turn on the redirection.
There may not be hard-coded file names to be written.
There was a problem hiding this comment.
I see, in this case, it should be redirected, so there must be a configuration file?
There was a problem hiding this comment.
Yes, it should be redirected, but there does not need to be a configuration file. Runtime configurability does not depend on configuration file.
| @@ -160,6 +160,12 @@ class MODMESH_PYTHON_WRAPPER_VISIBILITY WrapRApplication | |||
| { | |||
| return self.main()->pytext(); | |||
| }) | |||
| .def_property_readonly( | |||
There was a problem hiding this comment.
Things get exposed from C++ to Python need corresponding unit tests. At the moment I do not see the need to access it from Python.
| @@ -86,14 +95,19 @@ def get_appenv(name=None): | |||
| get_appenv(name='master') | |||
|
|
|||
|
|
|||
| def run_code(code): | |||
| def run_code(code, redirectStdOutFile=None, redirectStdErrFile=None): | |||
There was a problem hiding this comment.
This is not a right way to redirect buffered I/O in either Python or C++.
Change all applications to set the Python command line instead.
Major changes include: * The menubar initialization is moved from RApplication to RMainWindow. * Remove redundant code in RAction. * Main window title, resize, and show are exposed and moved to Python. * Make modmesh.view.app an object (instead of function) again.
* Make sure RMainWindow::setUp() is only set up once. * Populate app menu in RMainWindow::setUp(). * Keep RMainWindow::show() to Python.
No description provided.