Repository: unihd-cag/skillbridge (language: python)

Issue:
[SUPPORT]: order of execution
Hello, first of all I want to thanks you for this amazing tool.
Then here is my issue, I am working on converting some old skill code into python so I have to do it part by part. Hence, I'm working with the direct mode and I'm calling with pyRunScript a python script in the middle of other skill instruction. what seems to happened is that the python scripts is ended after other skill instruction that where after the pyRunScript instruction.
For a simple example with this code skill: 
```
(pyRunScript "test.py")
println("end ")
```
And the test.py code : 
```python
from skillbridge import Workspace
ws = Workspace.open(direct = True)
ws["println"]("toto")
ws.close()
```
The output that I get with that one is :
```
end 
t
toto
script finished with code 0
```
while the expected output would be : 
```
toto
script finished with code 0
end
t
```
Did I miss something in the documentation? or is there something that I can do to "force" the execution of the python script before the end of the skill script?

Best regards,


Discussion / hints:
Hi,

> Did I miss something in the documentation?

I believe we didn't document this behavior. `pyRunScript` effectively calls `ipcBeginProcess(...)`. That is a non-blocking operation, so the python script will run in the background.

You can however explicitly wait for the process to finish by using `ipcWait`. So in your case:

```lisp
(ipcWait (pyRunScript "test.py"))
println("end ")
```

should do the trick.

@TM90 maybe we should document this, or add a `block` flag to `pyRunScript`?

It could look like this: `pyRunScript("test.py" ?block t)`
Okay, thanks for the response! 
Regards,
> @TM90 maybe we should document this, or add a `block` flag to `pyRunScript`?
> 
> It could look like this: `pyRunScript("test.py" ?block t)`

I think adding the block flag would be a good addition

Fix patch:
diff --git a/docs/usage/quickstart.rst b/docs/usage/quickstart.rst
index 5a9011c..b7370bb 100644
--- a/docs/usage/quickstart.rst
+++ b/docs/usage/quickstart.rst
@@ -98,3 +98,13 @@ And you can pass CLI arguments to the script
 
     If used with the above code, this will print "geGetEditCellView()" to *stdout* and
     "cell view: 1234" to *stderr*.
+
+.. warning::
+
+    Running a python script with ``pyRunScript`` does **not wait** until the script
+    is finished. If you rely on side effects the script causes, you can block until
+    the script is finished.
+
+.. code-block:: lisp
+
+    pyRunScript "pathToScript.py" ?block t
diff --git a/skillbridge/server/python_server.il b/skillbridge/server/python_server.il
index fde2d78..2e82295 100644
--- a/skillbridge/server/python_server.il
+++ b/skillbridge/server/python_server.il
@@ -218,10 +218,12 @@ let((_filename _baseName _moduleFolder _logDirectory _executable _unbound)
     pyStartServer.logName = lsprintf("%s/skillbridge_skill.log" _logDirectory)
 
     putd('pyRunScript nil)
-    defun(pyRunScript (script @key (python "python") (args nil) "ttl")
-        let((command)
+    defun(pyRunScript (script @key (python "python") (args nil) (block nil) "ttlg")
+        let((command process)
             command = lsprintf("%s %s %s" python script buildString(args))
-            ipcBeginProcess(command "" '__pyOnData '__pyOnError '__pyOnFinishScript pyRunScript.logName)
+            process = ipcBeginProcess(command "" '__pyOnData '__pyOnError '__pyOnFinishScript pyRunScript.logName)
+
+            if(block then ipcWait(process) else process)
         )
     )
 


Test patch:
diff --git a/tests/script.py b/tests/script.py
new file mode 100644
index 0000000..e71e06c
--- /dev/null
+++ b/tests/script.py
@@ -0,0 +1,13 @@
+from ast import literal_eval
+from sys import argv
+from time import sleep
+
+from skillbridge import Symbol, Workspace
+
+ws = Workspace.open(direct=True)
+
+_, variable_name, value, delay = argv
+
+sleep(float(delay))
+
+ws['set'](Symbol(variable_name), literal_eval(value))
diff --git a/tests/test_integration.py b/tests/test_integration.py
index 8cfe8e3..f22afb8 100644
--- a/tests/test_integration.py
+++ b/tests/test_integration.py
@@ -1,9 +1,12 @@
+from pathlib import Path
+from time import sleep
 from warnings import warn
 
-from _pytest.python_api import raises
-from pytest import fixture, skip
+from pytest import fixture, raises, skip
 
-from skillbridge import LazyList, RemoteObject, RemoteTable, SkillCode, Symbol, Workspace
+from skillbridge import LazyList, RemoteObject, RemoteTable, SkillCode, Symbol, Var, Workspace
+
+here = Path(__file__).parent
 
 
 @fixture(scope='module')
@@ -214,3 +217,21 @@ def test_table_getattr_is_equivalent_to_symbol_lookup(ws: Workspace) -> None:
 
 def test_nil_t_nil_is_not_a_disembodied_property_list(ws: Workspace) -> None:
     assert ws["cdr"]([0, None, True, None]) == [None, True, None]
+
+
+def test_run_script_does_not_block(ws: Workspace) -> None:
+    variable = 'skillbridge_script_args'
+    ws['set'](Symbol(variable), 0)
+    assert ws['pyRunScript'](str(here / 'script.py'), args=(variable, '42', '0.25'))
+
+    assert ws['plus'](Var(variable), 1) == 1
+    sleep(1.0)
+    assert ws['plus'](Var(variable), 1) == 43
+
+
+def test_run_script_blocks_when_requested(ws: Workspace) -> None:
+    variable = 'skillbridge_script_args'
+    ws['set'](Symbol(variable), 0)
+    assert ws['pyRunScript'](str(here / 'script.py'), args=(variable, '42', '0.25'), block=True)
+
+    assert ws['plus'](Var(variable), 1) == 43