Need to add a correction: The new interact macro captures the expect-port for input if available, and uses it as the interact/output port too. This is unless an explicit interact/output port is passed as an optional argument, in which case the lines read from default current-input-port (using ice-9 readline) are then sent to this port instead.
Cheers, Daniel On Sun, 21 May 2023 at 23:26, Daniel Dinnyes <dinny...@gmail.com> wrote: > Hi everyone, > > It seems this hasn't garnered much attention. > > Nevertheless, meanwhile I've realised the code had a few bugs, in > particular around the backwards-compatibility flag. > > I've done a major rewrite: > > - fixed couple of bugs I found in the original implementation > - made implementation more robust, simpler, and backwards-compatible: > - added new expect-chars macro to support the new > non-backwards-compatible matcher procedure signature. > - removed need for expect-pass-char? backwards-compatibility > feature flag. expect and expect-strings macros work as in ice-9. > - improved interact macro, to allow specifying separate > expect/input and interact/output ports. > > The code can be found here > <https://github.com/dadinn/common/blob/feature/expect4/expect.scm>. > > Thanks, > Daniel > > > On Sat, 15 Apr 2023 at 16:10, Daniel Dinnyes <dinny...@gmail.com> wrote: > >> Hi everyone, >> >> I am new to this mailing list, and writing this because I have >> encountered some issues with ice-9 expect >> <https://git.savannah.gnu.org/cgit/guile.git/tree/module/ice-9/expect.scm> >> : >> >> The procedural expect macro has a serious problem: >> >> This syntax expects a procedure as the test expression, with 2 arguments >> (actual content read, and boolean EOF flag). The primary problem is that >> when the test expression is not a identifier bound to such a procedure, but >> instead an expression which creates such a procedure (aka "factory"), then >> the expression will be evaluated anew for each character read from >> expect-port. This is a serious problem: when constructing that procedure >> has significant computational costs, or side effects (eg. opening a port to >> a new log file), as it can be seen in my code here >> <https://github.com/dadinn/system-setup-tests/blob/expect-bug/run.scm#L476>. >> (This is a branch of some of my KVM-based E2E testing code, which uses the >> original expect macro implementation, just to demonstrate the problem). >> >> I had written a workaround for this problem, by binding the evaluation of >> the test expression outside the expansion of the expect macro (as it can be >> seen here >> <https://github.com/dadinn/system-setup-tests/blob/master/run.scm#L29>). >> The problem with this was, that it doesn't support the full capabilities of >> the original expect macro, and debilitates it by only allowing for a single >> conditional branch. Also, I haven't verified, but I suspicious it would >> possibly even break the behaviour for the => syntax of the expect-strings >> macro. >> >> To implement a more complete version of this (and also as an exercise to >> understand hygienic macros), I've attempted to rewrite my workaround using >> syntax-case. I've ran into some issues, and my implementation didn't >> expand as I expected. After some discussion on IRC, we've concluded >> <http://snailgeist.com/irc-logs/?message_id=136306> that this was >> because the current (ice-9 expect) implementation is written using >> unhygienic defmacro syntax, which doesn't work well together with the >> hygienic syntax-case. It was suggested I should rather rewrite expect >> completely to be hygienic instead. >> >> I've eventually completed the rewrite using syntax-case here >> <https://github.com/dadinn/common/blob/feature/expect/expect.scm>, and >> would be happy to contribute it back to the ice-9 module! >> I would point out the following about this implementation: >> >> 1. It works!!! I had some quite extensive E2E testing code written >> before here >> <https://github.com/dadinn/system-setup-tests/blob/develop/run.scm>, >> which I think really stretches the possibilities with (ice-9 expect). >> Switching to the new implementation was simple, and works flawlessly! >> 2. It is backwards compatible! I've gone extra lengths to make sure >> the code would be a backwards compatible, a drop-in replacement. >> 3. Introduced new feature for the procedural expect macro, such that >> the second argument of the procedure gets passed the currently read char, >> or #f if it was an EOF character. There is multiple reasons this is >> superior to passing just a boolean eof? flag argument: >> 1. the same logic can be implemented as with the eof? boolean, just >> by negating the condition >> 2. passing the currently read char avoids having to do the >> inefficient operation to retrieve the currently read char by >> cutting off >> the last character from the end of the content. Currently it is >> necessary >> to do this, if the logic wants something additional to be done with >> the >> current char (eg. as it can be seen in my code here >> >> <https://github.com/dadinn/system-setup-tests/blob/develop/run.scm#L200> >> ) >> 3. one such case of additional use is making the >> expect-char-proc property obsolete, as the predicate procedure >> is now able to execute such logic too, besides other more advanced >> logging >> scenarios (like it can be seen in my code here >> >> <https://github.com/dadinn/system-setup-tests/blob/develop/run.scm#L205>: >> here besides logging everything to STDOUT, and also to a main >> log-file, I >> also log for each expect macro call the contents it tries to compare >> against into a separately named minor log-file (useful for >> debugging), and >> also to ensure that we actually do not write to STDOUT when using >> expect on >> multiple parallel processes) >> 4. To ensure that everything stays backwards compatible, this >> new feature is only enabled if the new expect-pass-char? >> binding is set to #t in the lexical context (like here >> >> <https://github.com/dadinn/system-setup-tests/commit/21bddce3cd1c14f8834adaee4ff75f5b1b7a7b04>). >> Otherwise, by default this feature is disabled, and boolean eof? >> flag argument is passed just like before. >> 4. There is a better support for using default >> value/expressions for the various parameters the macros depend on in their >> lexical contexts. The old macro had to export default values defined in >> its >> module, so as to ensure the defaults are available. This either clutters >> the module's global context where (ice-9 expect) is used, or the >> defaults are not available when using something like ((ice-9 expect) >> #:select (expect expect-string)). The new defaults are calculated >> based on the lexical context where the macros are used, and expands into >> the correct default values when they are not defined. For example when >> expect-port is not defined in the lexical context, then the default value >> used is going to be the result of the (current-input-port) >> expression. I would like someone to review whether I've implemented this >> correctly, as it seems the current-input-port identifier default for >> expect-port gets captured in the expansion. Not sure why that happens. >> 5. The expect-strings macro supports the => syntax, which passes the >> output(s) of the test predicate to a procedure, instead of just evaluating >> a body. This is fully supported, but additionally, the => syntax now >> also works with the procedural expect macro too! I haven't verified if the >> original implementation did this too, but at least it wasn't described in >> the documentation! >> 6. As an added bonus, I have added a simplistic implementation for an >> interact macro. It uses threads to read from STDIO, and expects >> expect-port >> to be an input-output-port, so that it can output the lines read from >> STDIO >> into the same port. Not too advanced, as it doesn't support a full >> terminal >> interface, with autocomplete, etc... but works well enough for my use-case >> to interact with a KVM process via serial port. >> 7. mnieper said <http://snailgeist.com/irc-logs/?message_id=136211> >> that I am not using syntax-case idiomatically, and my style is more in the >> form of a syntax-rules-based state-machine. >> I am quite happy with how it is currently, but would be open to be >> convinced if there is a superior way of expressing all this. >> 8. I currently have various tests sprinkled around my implementation, >> to manually demonstrate expansions, and executions. >> Also, I have added some automated unit tests for the bind-locally >> helper procedure, which I used for checking whether parameters are defined >> in the lexical scope. >> 9. I am not happy with how I am managing my project structure, and >> how my modules can reference each other (having to call add-to-load-path >> everywhere). >> Also, I think the way I am using srfi-64 tests is a bit bothersome, >> as they always get executed when my modules are loaded, and I have to >> globally disable logging to files. >> I would like some recommendations how to do this better! >> >> If the community likes this implementation, I would be happy to >> apply/rebase my changes onto the official repo, and then we can go from >> there! >> >> Best regards, >> Daniel Dinnyes >> >> >>
