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

First VBScript implementation #624

Merged
merged 129 commits into from
Aug 15, 2024
Merged

First VBScript implementation #624

merged 129 commits into from
Aug 15, 2024

Conversation

OldLiu001
Copy link
Contributor

@OldLiu001 OldLiu001 commented Jan 28, 2023

Hello everyone, maintainers & contributors!

After more than half a year, I finally implement it.

During this period, I benefited a lot and felt the charm of Lisp language. Thank you for the project and its contributors.

Unfortunately, it seems that the makefile file and python script cannot work on Windows, and Vbscript cannot run in linux.

But I have manually tested all the examples in the tests folder and make sure I passed them (run mal impl in VBScript are also passed).

Nevertheless, I still want to contribute my learning process and achievements to this project to attract future generations.

The readme file has already updated for others to run my script.

@OldLiu001 OldLiu001 changed the title First VBScript implement First VBScript implementation Jan 29, 2023
@cy20lin
Copy link

cy20lin commented Jul 7, 2023

Hi @OldLiu001

Congrats to you for making your own VBScript implementation.

I made a pull request (#640) which enable the runtest.py script to run mal implementation tests on Windows. You may refer to that PR and see if the script helps.

@OldLiu001
Copy link
Contributor Author

OldLiu001 commented Jul 10, 2023

Hi @OldLiu001

Congrats to you for making your own VBScript implementation.

I made a pull request (#640) which enable the runtest.py script to run mal implementation tests on Windows. You may refer to that PR and see if the script helps.

Thank you for your contribution.

I test my impl with this script (Batch File):

@echo off
pushd "%~dp0"
rem goto p
for %%a in (
	step0_repl
	step1_read_print
	step2_eval
	step3_env
	step4_if_fn_do
	step5_tco
	step6_file
	step7_quote
	step8_macros
	step9_try
	stepA_mal
) do (
	echo @pushd "%%~dp0" ^& @cscript -nologo %%a.vbs > .\impls\vbs\run_%%a.cmd
	python runtest.py --rundir "impls\vbs" --test-timeout 1800 --deferrable --optional --no-pty "..\tests\%%a.mal" "run_%%a.cmd"
	del .\impls\vbs\run_%%a.cmd
	rem pause
)
exit

It passed all the tests.

runtest.log

@OldLiu001 OldLiu001 force-pushed the master branch 2 times, most recently from b56ec95 to 96cda20 Compare June 15, 2024 05:54
Copy link
Owner

@kanaka kanaka left a comment

Choose a reason for hiding this comment

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

Hello @OldLiu001 sorry for the very slow reply (I've been unable to work on this project for the past couple of years but I can again). If you're still interested in getting this merged upstream I would ask that you do the following:

  • Rebase the code onto the current HEAD
  • The recommended process has changed slightly: the eval_ast (EvaluateAST) is no longer seperate from eval (Evaluate) and macroexpand has been simplified (integrated into eval and no longer a separate function). It would be good if you could update to follow the new structure.
  • Add windows runner support to the Github Actions CI process. This will mean modifying:
  • Add you implementation to Makefile.impls and to IMPLS.yml and make sure that the CI tests all pass for your implementation.

Copy link
Contributor Author

@OldLiu001 OldLiu001 left a comment

Choose a reason for hiding this comment

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

Thank you for your reply.
I'm glad you've been available to handle these pull requests lately, and I still look forward to my code being merged upstream to help more learners.

I have made the corresponding changes according to your suggestions.

I also encountered the following problems:

  • Github Action calls ci.sh normally, ci.sh calls runtest.py normally, but I found that runtest.py could not call the bash script impls/vbs/run properly, showing not a runable program or batch file.
  • I tried to append bash to the header of the command line before python called impls/vbs/run (like bash ../vbs/run), but the call still failed, and the Github Action log showed that bash command needed WSL installed before it could be used.
  • It seems that in the windows environment of Github Action, bash scripts cannot call bash again after calling another scripting environment.
  • I tried to search the Internet, but there was no discussion of the issue.
  • So I wrote a batch script run.cmd that does exactly the same thing as the bash script run, and modified the logic associated with runtest.py to default to calling the batch script run.cmd instead of the bash script run on windows.
  • Because the test run_argv_test.sh in step 6 requires calling the bash script run, I kept both versions of the script (run & run.cmd).

With the above actions, I successfully completed testing my VBScript implementation on the Windows environment of Github Action.

You can find details of the tests for my implementation at the bottom of the test results.

Copy link
Owner

@kanaka kanaka left a comment

Choose a reason for hiding this comment

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

@OldLiu001 Apart from the runtest.py changes, everything else looks good. The runtest.py changes to support Windows are pretty intrusive. The program is already pretty hard to maintain and reason about.

Would you be willing to try WSL for the Windows workflow job? There is an existing action that will install WSL https://github.com/Vampire/setup-wsl. I would like to see if that would allow runtest.py to work without needing to completely rework it to run on Windows. I suspect that it should work without the run.cmd too. If you can get this working under WSL without the major runtest.py changes, then I would consider this ready for merging.

Also, did you see my comment about the incremental process/layout changing (eval_ast and macroexpand are no longer separate functions). It's not required for getting this merged, but it would be good if you could look into that at some point.

@OldLiu001
Copy link
Contributor Author

@kanaka Thank you for your advice.

I just tried the following:

  • Restore runtest.py to the unupdated version.
  • Install an WSL on my local machine.
  • Test runtest.py with and without --no-pty argument.

Then I got:

$ env STEP=step0_repl MAL_IMPL=vbs ../../runtest.py  --deferrable --optional  ../tests/step0_repl.mal -- ../vbs/run
Did not receive one of following prompt(s): ['[^\\s()<>]+> ']
    Got      : '\x1b[6n'

Exception: SystemExit(1,)
Output before exception:

^[[30;1R
$ env STEP=step0_repl MAL_IMPL=vbs ../../runtest.py  --deferrable --optional --no-pty ../tests/step0_repl.mal -- ../vb
s/run
Testing basic string
TEST: 'abcABC123' -> ['',abcABC123] -> FAIL (line 3):
    Expected : '.*\nabcABC123'
    Got      : 'abcABC123'
Testing string containing spaces
TEST: 'hello mal world' -> ['',hello mal world] -> FAIL (line 7):
    Expected : '.*\nhello\\ mal\\ world'
    Got      : 'hello mal world'
Testing string containing symbols
TEST: '[]{}"\'* ;:()' -> ['',[]{}"'* ;:()] -> FAIL (line 11):
    Expected : '.*\n\\[\\]\\{\\}\\"\\\'\\*\\ \\;\\:\\(\\)'
    Got      : '[]{}"\'* ;:()'
Test long string
TEST: 'hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"\'* ;:() []{}"\'* ;:() []{}"\'*)' -> ['',hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"'* ;:() []{}"'* ;:() []{}"'*)] -> FAIL (line 16):
    Expected : '.*\nhello\\ world\\ abcdefghijklmnopqrstuvwxyz\\ ABCDEFGHIJKLMNOPQRSTUVWXYZ\\ 0123456789\\ \\(\\;\\:\\(\\)\\ \\[\\]\\{\\}\\"\\\'\\*\\ \\;\\:\\(\\)\\ \\[\\]\\{\\}\\"\\\'\\*\\ \\;\\:\\(\\)\\ \\[\\]\\{\\}\\"\\\'\\*\\)'
    Got      : 'hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"\'* ;:() []{}"\'* ;:() []{}"\'*)'
Non alphanumeric characters
TEST: '!' -> ['',!] -> FAIL (line 20):
    Expected : '.*\n\\!'
    Got      : '!'
TEST: '&' -> ['',&] -> FAIL (line 22):
    Expected : '.*\n\\&'
    Got      : '&'
TEST: '+' -> ['',+] -> FAIL (line 24):
    Expected : '.*\n\\+'
    Got      : '+'
TEST: ',' -> ['',,] -> FAIL (line 26):
    Expected : '.*\n\\,'
    Got      : ','
TEST: '-' -> ['',-] -> FAIL (line 28):
    Expected : '.*\n\\-'
    Got      : '-'
TEST: '/' -> ['',/] -> FAIL (line 30):
    Expected : '.*\n\\/'
    Got      : '/'
TEST: '<' -> ['',<] -> FAIL (line 32):
    Expected : '.*\n\\<'
    Got      : '<'
TEST: '=' -> ['',=] -> FAIL (line 34):
    Expected : '.*\n\\='
    Got      : '='
TEST: '>' -> ['',>] -> FAIL (line 36):
    Expected : '.*\n\\>'
    Got      : '>'
TEST: '?' -> ['',?] -> FAIL (line 38):
    Expected : '.*\n\\?'
    Got      : '?'
TEST: '@' -> ['',@] -> FAIL (line 40):
    Expected : '.*\n\\@'
    Got      : '@'
TEST: '^' -> ['',^] -> FAIL (line 43):
    Expected : '.*\n\\^'
    Got      : '^'
TEST: '_' -> ['',_] -> FAIL (line 45):
    Expected : '.*\n\\_'
    Got      : '_'
TEST: '`' -> ['',`] -> FAIL (line 47):
    Expected : '.*\n\\`'
    Got      : '`'
TEST: '~' -> ['',~] -> FAIL (line 49):
    Expected : '.*\n\\~'
    Got      : '~'
------- Optional Functionality --------------
------- (Not needed for self-hosting) -------
Non alphanumeric characters
TEST: '#' -> ['',#] -> SOFT FAIL (line 58):
    Expected : '.*\n\\#'
    Got      : '#'
TEST: '$' -> ['',$] -> SOFT FAIL (line 60):
    Expected : '.*\n\\$'
    Got      : '$'
TEST: '%' -> ['',%] -> SOFT FAIL (line 62):
    Expected : '.*\n\\%'
    Got      : '%'
TEST: '.' -> ['',.] -> SOFT FAIL (line 64):
    Expected : '.*\n\\.'
    Got      : '.'
TEST: '|' -> ['',|] -> SOFT FAIL (line 66):
    Expected : '.*\n\\|'
    Got      : '|'

FAILURES:
FAILED TEST (line 3): abcABC123 -> ['',abcABC123]:
    Expected : '.*\nabcABC123'
    Got      : 'abcABC123'
FAILED TEST (line 7): hello mal world -> ['',hello mal world]:
    Expected : '.*\nhello\\ mal\\ world'
    Got      : 'hello mal world'
FAILED TEST (line 11): []{}"'* ;:() -> ['',[]{}"'* ;:()]:
    Expected : '.*\n\\[\\]\\{\\}\\"\\\'\\*\\ \\;\\:\\(\\)'
    Got      : '[]{}"\'* ;:()'
FAILED TEST (line 16): hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"'* ;:() []{}"'* ;:() []{}"'*) -> ['',hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"'* ;:() []{}"'* ;:() []{}"'*)]:
    Expected : '.*\nhello\\ world\\ abcdefghijklmnopqrstuvwxyz\\ ABCDEFGHIJKLMNOPQRSTUVWXYZ\\ 0123456789\\ \\(\\;\\:\\(\\)\\ \\[\\]\\{\\}\\"\\\'\\*\\ \\;\\:\\(\\)\\ \\[\\]\\{\\}\\"\\\'\\*\\ \\;\\:\\(\\)\\ \\[\\]\\{\\}\\"\\\'\\*\\)'
    Got      : 'hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"\'* ;:() []{}"\'* ;:() []{}"\'*)'
FAILED TEST (line 20): ! -> ['',!]:
    Expected : '.*\n\\!'
    Got      : '!'
FAILED TEST (line 22): & -> ['',&]:
    Expected : '.*\n\\&'
    Got      : '&'
FAILED TEST (line 24): + -> ['',+]:
    Expected : '.*\n\\+'
    Got      : '+'
FAILED TEST (line 26): , -> ['',,]:
    Expected : '.*\n\\,'
    Got      : ','
FAILED TEST (line 28): - -> ['',-]:
    Expected : '.*\n\\-'
    Got      : '-'
FAILED TEST (line 30): / -> ['',/]:
    Expected : '.*\n\\/'
    Got      : '/'
FAILED TEST (line 32): < -> ['',<]:
    Expected : '.*\n\\<'
    Got      : '<'
FAILED TEST (line 34): = -> ['',=]:
    Expected : '.*\n\\='
    Got      : '='
FAILED TEST (line 36): > -> ['',>]:
    Expected : '.*\n\\>'
    Got      : '>'
FAILED TEST (line 38): ? -> ['',?]:
    Expected : '.*\n\\?'
    Got      : '?'
FAILED TEST (line 40): @ -> ['',@]:
    Expected : '.*\n\\@'
    Got      : '@'
FAILED TEST (line 43): ^ -> ['',^]:
    Expected : '.*\n\\^'
    Got      : '^'
FAILED TEST (line 45): _ -> ['',_]:
    Expected : '.*\n\\_'
    Got      : '_'
FAILED TEST (line 47): ` -> ['',`]:
    Expected : '.*\n\\`'
    Got      : '`'
FAILED TEST (line 49): ~ -> ['',~]:
    Expected : '.*\n\\~'
    Got      : '~'
SOFT FAILED TEST (line 58): # -> ['',#]:
    Expected : '.*\n\\#'
    Got      : '#'
SOFT FAILED TEST (line 60): $ -> ['',$]:
    Expected : '.*\n\\$'
    Got      : '$'
SOFT FAILED TEST (line 62): % -> ['',%]:
    Expected : '.*\n\\%'
    Got      : '%'
SOFT FAILED TEST (line 64): . -> ['',.]:
    Expected : '.*\n\\.'
    Got      : '.'
SOFT FAILED TEST (line 66): | -> ['',|]:
    Expected : '.*\n\\|'
    Got      : '|'

TEST RESULTS (for ../tests/step0_repl.mal):
    5: soft failing tests
   19: failing tests
    0: passing tests
   24: total tests

The results show that when WSL communicates with Windows native console programs, some unexpected behavior may occur, leading to test failure.

In order to be compatible with Windows, it seems that runtest.py must make some big changes.

I also agree with you that patching runtest.py and repeating scripts run with run.cmd is very inelegant. I'm wondering if there is a more elegant way to do this, and if you have any suggestions or ideas, please feel free to let me know.

Besides, I have taken note of incremental process / layout changing problem. But I think we should finish our discussion of the current issue and then consider some relative details.

@kanaka
Copy link
Owner

kanaka commented Aug 8, 2024

Can you run those tests again with --debug-file debug.log added to the runtest command? I would be interested to see what the content of that would be for each case (for the second one, you don't need to post the full file, just the top 30 lines or so). Might indicate what is going wrong and hopefully there is a simple way to work around it.

@OldLiu001
Copy link
Contributor Author

OldLiu001 commented Aug 9, 2024

@kanaka I did the following tests:

  • Use the unupdated version of runtest.py.
  • Retest the vbs impl to get the log file.
  • Test the bash impl in WSL for comparison.

The results of the bash impl + WSL test are as follows.

$ env STEP=step0_repl MAL_IMPL=bash ../../runtest.py  --deferrable --optional --debug-file wsl_bash_withpty.log ../tests/step0_repl.mal -- ../bash/run
...
TEST RESULTS (for ../tests/step0_repl.mal):
    0: soft failing tests
    0: failing tests
   24: passing tests
   24: total tests

$ env STEP=step0_repl MAL_IMPL=bash ../../runtest.py  --deferrable --optional --no-pty --debug-file wsl_bash_nopty.log
 ../tests/step0_repl.mal -- ../bash/run
Did not receive one of following prompt(s): ['[^\\s()<>]+> ']
    Got      : ''

Exception: SystemExit(1,)
Output before exception:

Log files are as follows:

$ head wsl_vbs_nopty.log
user> abcABC123
user> hello mal world
user> []{}"'* ;:()
user> hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"'* ;:() []{}"'* ;:() []{}"'*)
user> !
user> &
user> +
user> ,
user> -
user> /

$ od -t x1 wsl_vbs_withpty.log
0000000 1b 5b 36 6e
0000004

$ od -t x1 wsl_bash_nopty.log
0000000

$ head wsl_bash_withpty.log
user> abcABC123
abcABC123
user> hello mal world
hello mal world
user> []{}"'* ;:()
[]{}"'* ;:()
user> hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"'* ;:() []{}"'* ;:() []{}"'*)
hello world abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789 (;:() []{}"'* ;:() []{}"'* ;:() []{}"'*)
user> !
!

wsl_test_logs.zip

So far I have reached the following conclusions:

  • WSL + bash impl + pty works normally, and WSL + vbs impl + nopty may work normally after runtest.py is modified.
  • log of WSL + vbs impl + pty has 4 bytes, and log of WSL + bash impl + nopty is an empty file.
  • It seems that WSL + bash impl + pty will repeat the standard input in the standard output, but WSL + vbs impl + nopty will not.

@dubek
Copy link
Collaborator

dubek commented Aug 9, 2024

1b 5b 36 6e is an ANSI escape sequence: ESC [ 6 n . From https://man7.org/linux/man-pages/man4/console_codes.4.html :

  ECMA-48 Status Report Commands

  ESC [ 5 n
         Device status report (DSR): Answer is ESC [ 0 n (Terminal
         OK).

  ESC [ 6 n
         Cursor position report (CPR): Answer is ESC [ y ; x R,
         where x,y is the cursor location.

Not sure exactly how to continue, but maybe this will help.

(welcome back @kanaka , glad you're okay. got me a bit worried, I must say.)

@OldLiu001
Copy link
Contributor Author

Let me briefly summarize the above discussion:

  • According to my previous discussion with @cy20lin in Support runtest.py to work on Windows when no_pty=True #640, the runtest.py modified by @cy20lin has been fully tested on the Windows platform, but the description of various details of the changes in runtest.py is lacking.
  • I tried to use WSL to achieve Windows platform compatibility without changing the main logic of the project's original runtest.py, but it seems that things are not as simple as we thought. Maybe we need to re-encounter and solve all the problems that @cy20lin encountered and solved before.
  • The content discussed in this thread has deviated from its original intention of contributing an implementation of a new language, and too many other changes have been added.

My personal advice:

  • I personally recommend using @cy20lin's solution for the time being and completing support for Windows platform first.
  • Later we can discuss in a new thread whether we can optimize support for the Windows platform to make our workflow more elegant.

@OldLiu001
Copy link
Contributor Author

OldLiu001 commented Aug 9, 2024

@kanaka Regarding the structural changes you mentioned in merging 'EvaluateAST' and 'MacroExpand' into 'Evaluate', I went to #587 , and since it's been a long time since I finished my vbs implementation, I went back to my code structure.

I found it interesting that when I was writing code, I already realized that normal functions and macros were identical in evaluation (that is, they were identical except for whether the parameters were evaluated), and I used the same class to represent both structures. I implemented the evaluation procedure as two methods of the 'MalProcedure' class, 'Apply' and 'ApplyWithoutEval'. I even added a new method, 'MacroApply'. It differs from 'ApplyWithoutEval' only in that it checks whether 'Procedure' is a macro at its head, and the logic I implemented to distinguish between functions and macros was to use a different class attribute as a tag.

I even unified the interfaces for 'vbs functions' and 'mal functions' and 'mal macro' calls (although the object-oriented implementation of vbs has no concept of interfaces, but it is a dynamically typed language, and two classes can be considered to implement the same interface as long as they have the same properties and methods). This has the advantage of using the same upper level code for processing.

Regarding evalast merging into eval, I think this is really a good change because evalast is only called once in eval, and there is no need to implement it as a separate function. And it does integrate the logic of the code. But if you look at my implementation, you'll see that my TCO implementation is very elegant (forgive me for bragging here, lol). The core idea of my tco implementation is that if the last step a function takes is to call another function and then return to the previous layer, then we can swap the order of the two steps, that is, return to the previous layer before making the function call. I use lazy evaluation to delay EVAL in tail recursion scenarios, so separating evalast and eval does not increase the call stack or cause my code to become verbose and uncontrollable.

In summary, I found that some of the ideas for merging code structures were already present in my implementation, and some didn't seem too bad for my implementation, so I thought it would be best to keep the current version.

Also something I discovered while writing my implementation is that if you switch the order in which some functions are implemented in the flow, many functions can be implemented with other functions, that is, the code can contain more parts that implement mal using mal itself. Using the mal language itself to implement mal's other functions may cause the code to run less efficiently, but I think that's cool, right?

@kanaka
Copy link
Owner

kanaka commented Aug 9, 2024

@dubek Thanks! I'm glad to be able to spend time on this again.

@kanaka
Copy link
Owner

kanaka commented Aug 9, 2024

@OldLiu001 runtest.py is already too complicated and a maintenance burden so I'm not willing to make it more complicated like proposed in #640. It would be great to support Windows implementations but I'm not willing to complicate runtest.py that much to support a small handful of implementations. The software landscape has changed and it's pretty rare for languages to only support Windows any more (vbscript being an obvious exception).

Anyways, thank for the detailed output and good thought to compare with bash. That has given me an idea for a minor change to runtest to support Windows+WSL in--no-pty mode. The reason it's failing is because the vbscript implementation is not echo'ing the input/test form back to the terminal. Please try this tweak to runtest.py and see if it changes things: https://github.com/kanaka/mal/compare/master...runtest-no-pty-no-echo?diff=unified&w=.

If it's still broken, I would be interested in the the same debug output from vbscript and bash as before. I'm pretty confident that we can get this to work without the full refactoring of runtest.py.

@kanaka
Copy link
Owner

kanaka commented Aug 9, 2024

Also something I discovered while writing my implementation is that if you switch the order in which some functions are implemented in the flow, many functions can be implemented with other functions, that is, the code can contain more parts that implement mal using mal itself. Using the mal language itself to implement mal's other functions may cause the code to run less efficiently, but I think that's cool, right?

Yeah, that's true that there are a number of functions that could be implemented in mal itself (and some special forms that could be implemented in mal itself using macros). But the primary goal of mal is pedagogical (learning tool). That goal has two parts: 1. learning about Lisp by implementing a Lisp interpreter, 2. learning about the host language by implementing a significant application in that language. The choices about the order of the steps and which functions are implemented in mal vs in the host language is a balance between those two learning goals.

In each step I try and introduce a core Lisp concept and also leverage some new aspects of the host language. If the goal was only learning Lisp then the order of steps 5-A would be very different: macros and try/cache would be very early so that those could be used to implement many of the core functions and some of the special forms directly in mal itself. But the current order and when/how functions are implemented is trying to balance those two learning goals. For example, not is implemented as a function in mal itself in step 4. This is a weird choice if the goal of mal were primarily to implement a Lisp interpreter. However, it's serves a really useful learning goal to show early on that you can use mal to implement parts of mal and it enables better testing of the fn* form at that point in the flow.

@OldLiu001
Copy link
Contributor Author

@kanaka I did the following tests:

  • Use runtest.py you lately provided.
  • Run make "test^vbs^step0", it passed all the test smoothly.
  • Run make "test^vbs^step1", I test it many times and make sure it will stuck in first Regular Expression check.
$ env STEP=step1_read_print MAL_IMPL=vbs ../../runtest.py  --deferrable --optional --no-pty --debug-file vbs_step1.log ../tests/step1_read_print.mal -- ../vbs/run

... 

TEST: '"}"' -> ['',"}"] -> SUCCESS
TEST: '"~"' -> ['',"~"] -> SUCCESS
TEST: '"!"' -> ['',"!"] -> SUCCESS
Testing reader errors
TEST: '(1 2' -> ['.*(EOF|end of input|unbalanced).*',] -> TIMEOUT (line 142)

Exception: TestTimeout('TIMEOUT (line 142)',)
Output before exception:
user> Exception: unbalanced parentheses.

make: *** [Makefile:238: test^vbs^step1] Error 1

$ tail vbs_step1.log
user> "["
user> "]"
user> "^"
user> "_"
user> "`"
user> "{"
user> "}"
user> "~"
user> "!"
user> user> Exception: unbalanced parentheses.

vbs_step1.log

@OldLiu001
Copy link
Contributor Author

If it's still broken, I would be interested in the the same debug output from vbscript and bash as before. I'm pretty confident that we can get this to work without the full refactoring of .runtest.py

$  env STEP=step1_read_print MAL_IMPL=bash ../../runtest.py  --deferrable --optional --debug-file bash_step1.log ../te
sts/step1_read_print.mal -- ../bash/run

...


TEST RESULTS (for ../tests/step1_read_print.mal):
    0: soft failing tests
    0: failing tests
  118: passing tests
  118: total tests

Part of bash_step1.log:

user> 1
1
user> 7
7
user>   7   
7

...(skip)

user> "~"
"~"
user> "!"
"!"
user> (1 2
Error: expected ')', got EOF
user> [1 2
Error: expected ']', got EOF
user> "abc
Error: expected '"', got EOF

...(skip)

user> 1;\\\
1
user> 1;`
1
user> 1; &()*+,-./:;<=>?@[]^_{|}~
1
user> 

(EOF)

bash_step1.log

@kanaka
Copy link
Owner

kanaka commented Aug 9, 2024

@OldLiu001 looks like runtest is now working for vbs using --no-pty. That new problem appears to be because vbs errors (to stderr) are out of order with normal prints (to stdout). In other words, the output from that vbs as read by runtest was:

user> user> Exception: unbalanced parentheses.

but runtest is expecting them to be in correct order (the order the implementation wrote them):

user> Exception: unbalanced parentheses. user>

This likely means that stderr is buffered and stdout is not. That's pretty typical even in Linux/macos. That's one of the things that the pty mode addresses (in addition to allowing line editing i.e. libedit/GNU readline, to be tested properly). There are a few ways to address that:

  • get the vbs implementation to flush stderr after writing to it. most languages have some way to do this although it's possible vbscript does not (I didn't find anything with a couple of quick good searches)
  • wrap the vbs implementation with stdbuf -o0 -e0 cscript .... Not 100% certain this will work in calls to windows programs but this would work in linux/macos to unbuffer both stdout and stderr.
  • if neither of those are workable, provide a command line option for your implementation that writes errors to stdout instead of stderr if the interpreter is launched with that option.
  • switch all the Stderr to Stdout (simplest change but it is in some ways a regression in functionality of the implementation). There are a number of other implementations that print errors to stdout so it wouldn't be that unusual.

@OldLiu001
Copy link
Contributor Author

@kanaka Thank you for your precise point of view.

I wrote test_stderr_buffered.mal for testing:

$ cat test_stderr_buffered.mal
"a"
(1 2
"c"

$ cscript.exe -nologo step1_read_print.vbs < test_stderr_buffered.mal
user> "a"
user> user> Exception: unbalanced parentheses.
"c"
user>

The results show that there is indeed a problem with standard error streams being buffered, and your sense is keen!

I looked through the vbs manual and found no information about refreshing the output cache.

Then I test it on cmd.exe without WSL participate in:

D:\@PubCodes\mal\impls\vbs>cscript -nologo step1_read_print.vbs < test_stderr_buffered.mal
user> "a"
user> Exception: unbalanced parentheses.
user> "c"
user>
D:\@PubCodes\mal\impls\vbs>

I was surprised to find that the results were returned in the correct order.

Next I tried to fix it with:

$ stdbuf -o0 -e0 cscript.exe -nologo step1_read_print.vbs < test_stderr_buffered.mal
user> "a"
user> user> Exception: unbalanced parentheses.
"c"
user>

But it didn't work.

Combined with the above information, please feel free to advise.

I will also continue to search for information from the Internet, and if the cache problem is still not solved, then I will consider using StdOut instead of all StdErr.

@OldLiu001
Copy link
Contributor Author

OldLiu001 commented Aug 10, 2024

I avoided the buffered stderr problem by introducing cmd.exe as a mediator and merging the error output stream into the standard output stream within it.

impls/vbs/run

#!/bin/bash
cmd.exe /c "cscript -nologo $(dirname $0)/${STEP:-stepA_mal}.vbs "${@}" 2>&1"

Later, I found that introducing cmd as an intermediary would cause the parameter tests for step6 to fail, so I removed it again and changed the entire StdErr in my implementation to StdOut.

Now we have a new problem where runtest.py gets stuck when it encounter a test sample of (result ignored).
make "test^vbs^step1" (partial)

TEST: '({})' -> ['',({})] -> SUCCESS
Testing read of comments
TEST: ' ;; whole line comment (not an exception)' -> ['',] -> TIMEOUT (line 226)

Exception: TestTimeout('TIMEOUT (line 226)',)
Output before exception:
user>
make: *** [Makefile:238: test^vbs^step1] Error 1

make "test^vbs^stepA" (partial)

Testing readline
TEST: '(readline "mal-user> ")' -> ['',] -> TIMEOUT (line 9)

Exception: TestTimeout('TIMEOUT (line 9)',)
Output before exception:
mal-user>
make: *** [Makefile:238: test^vbs^stepA] Error 1
$ tail vbs_step1.log
user> {}
user> {"abc" 1}
user> {"a" {"b" 2}}
user> {"a" {"b" {"c" 3}}}
user> {"a" {"b" {"cde" 3}}}
user> {"a1" 1 "a2" 2 "a3" 3}
user> {:a {:b {:cde 3}}}
user> {"1" 1}
user> ({})
user> user>
$ tail vbs_stepA.log
Mal [VBScript]
user> mal-user>

vbs_step1.log vbs_stepA.log

Again, the Bash impl output for debug:
bash_step1.log (partial)

user> ({})
({})
user>  ;; whole line comment (not an exception)
../bash/printer.sh: line 60: declare: yes: not found
user> 1 ; comment after expression
1
user> 1; comment after expression
1

bash_stepA.log (partial)

Mal [bash]
user> (readline "mal-user> ")
mal-user> "hello"
"\"hello\""
user> (= "something bogus" *host-language*)
false

bash_step1.log bash_stepA.log

@OldLiu001
Copy link
Contributor Author

@kanaka The WSL environment on Github Action has been set up and the results are consistent with what I tested locally. I can already see the dawn of victory!

Finally get rid of the extra run.cmd lol.

@OldLiu001
Copy link
Contributor Author

OldLiu001 commented Aug 13, 2024

I tried:

        res = r.read_to_prompt(['\r\n[^\\s()<>]+> ', '\n[^\\s()<>]+> '],
                                timeout=args.test_timeout)

->

        res = r.read_to_prompt(['\r\n[^\\s()<>]+> ', '\n[^\\s()<>]+> ', '[^\\s()<>]+> '],
                                timeout=args.test_timeout)

Although it will not continue to stuck in the result ignored tests, it will cause misalignment of the output, which will cause some correct results to be mistaken for errors.

More logs:

$ cscript.exe -nologo step1_read_print.vbs < ../tests/step1_read_print.mal
... (ignored)

user> user> {}                                               <-- Synchronize with the start position of the log above
user> user> {"abc" 1}
user> user> {"a" {"b" 2}}
user> user> {"a" {"b" {"c" 3}}}
user> user> {"a" {"b" {"cde" 3}}}
user> user> user> user> {"a1" 1 "a2" 2 "a3" 3}
user> user> {:a {:b {:cde 3}}}
user> user> {"1" 1}
user> user> ({})
user> user> user> user> user> 1                              <-- runtest.py will stuck in this line
user> user> 1
user> user> user> user> (deref a)
user> user> user> user> a:
user> user> user> user> user> user> user> user> user> (with-meta [1 2 3] {"a" 1})
user> user> user> user> user> user> "\n"

... (ignored)

vbs_step1_output_full.log

$ cscript.exe -nologo stepA_mal.vbs < ../tests/stepA_mal.mal
Mal [VBScript]
user> user> user> user> user> user> user> user> user> mal-user> "\"hello\""     <-- runtest.py will stuck in this line
user> user> user> user> user> user> user> false
user> user> user> user> user> user> user> user> user> user> user> user> (atom {"+" #<function>})

... (ignored)

vbs_stepA_output_full.log

Not sure how to fix it, and would like your comments and suggestions.

@OldLiu001 OldLiu001 requested a review from kanaka August 13, 2024 03:03
@kanaka
Copy link
Owner

kanaka commented Aug 13, 2024

I think both failures are related to when newline are being printed out and the fact that when in test mode the user's input is not being echo'd (which would include a final newline).

For example, I think the first failure is caused by this check in your REPL routine:

				If strRes <> "" Then
					WScript.Echo strRes
				End If

I think that is preventing a newline in cases where the eval result is empty.

For the second failure related to the readline core function, I suspect there is again a newline issue but I'm not exactly sure from the result what's going on. Can you try adding a newline prior to every prompt? Something like this:

WScript.StdOut.Write "\nuser> "

You won't necessarily want that for interactive repl usage, but it might be enlightening to figure out what's going on with the tests.

@OldLiu001
Copy link
Contributor Author

OldLiu001 commented Aug 14, 2024

@kanaka Accurate insight!
Now that I've figured out the origin of all the problems, I've decided not to change runtest.py at all, but instead to encapsulate an IOWrap class in vbs, which has the option of choosing a different Repeat Form internally and whether to merge the error output stream into the standard output stream.
The rest of the code calls this IOWrap class, so we avoid code redundancy.
Externally, I passed several environment variables inside the run script to both WSL and vbs to regulate the behavior of the vbs implementation during testing.
Based on the above steps, the problem has been solved almost perfectly, both in terms of not changing runtest.py at all and in terms of the elegance of the code logic.
Thank you for your patient advice.
I would also like to thank @cy20lin and @dubek for their attention and selfless help.

@OldLiu001 OldLiu001 reopened this Aug 15, 2024
@OldLiu001
Copy link
Contributor Author

@kanaka I forcibly overwrote my master branch using the kanaka/mal/master branch, then cherry picked all vbs related commits from the previous records and applied them to the master branch.
That should be all right this time.

@kanaka kanaka merged commit b57cd56 into kanaka:master Aug 15, 2024
222 of 224 checks passed
@kanaka
Copy link
Owner

kanaka commented Aug 15, 2024

@OldLiu001 Merged! Thanks for your work on this and pushing through even though it was tedious. Hopefully any future implementations that support Windows will have a smoother path because they can refer to what you did to get things working. Any plans for future implementations? Want to try getting the powershell implementation running/testing in Windows (i.e. adding a new powershell line to IMPLS.yaml for windows)?

@OldLiu001
Copy link
Contributor Author

OldLiu001 commented Aug 15, 2024

@kanaka Thank you for your patience.
My next plan is to try to complete an implementation using batch scripts at my leisure.
Regarding the powershell implementation you mentioned, I often use the output of the powershell implementation as a reference when writing the vbs implementation, so it should not be a problem to run on windows.
However, in order for runtest.py to work, we may also need to wrap some interfaces like the io.vbs of vbs implementation to complete the switch between different output repetition patterns.

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

Successfully merging this pull request may close these issues.

4 participants