Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Internal improvement: Properly time REPL inputs #5479

Merged
merged 5 commits into from
Sep 7, 2022
Merged

Conversation

cds-amal
Copy link
Member

@cds-amal cds-amal commented Aug 22, 2022

Currently, runInREPL sends all queued commands to the child process on the first detected REPL prompt and then closes the input stream, which terminates the child process (bad).

This PR reworks the logic to send one queued command per REPL prompt.

@cds-amal cds-amal marked this pull request as ready for review August 22, 2022 16:27
@cds-amal cds-amal changed the title Wait for it Internal improvement: Properly time REPL inputs Aug 22, 2022
config,
executableCommand: "develop",
displayHost: "develop"
});
output = logger.contents();
});

it("Sends multiple commands to REPL", () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I landed on placing a test here because it's quicker to piggyback in this scenario than breakout another file to validate our testing library. Please let me know if you think this belongs elsewhere.

// seems safe to escape parens only, as the readyprompt is constructed from
// [a-zA-Z] strings and wrapping parens.
const escapedPrompt = readyPrompt.replace("(", "\\(").replace(")", "\\)");
const readyPromptRex = new RegExp(`^${escapedPrompt}`, "gm");
Copy link
Contributor

@sukanyaparashar sukanyaparashar Aug 22, 2022

Choose a reason for hiding this comment

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

Sorry @cds-amal if it is a silly question, but what this "gm" flag does? "g" is for global search and "m" is multi-line search if I am understanding correctly.

Copy link
Member Author

@cds-amal cds-amal Aug 22, 2022

Choose a reason for hiding this comment

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

Yes, "g" is for global, and "m" is for multiline. It means the meta characters, ^, and $ will match the beginning and end of each line instead of the beginning and end of the input string (which can contain multiple lines)

if (readyPromptRex.test(outputBuffer)) {
// set outputBuffer to remaining segment after final match
const segments = outputBuffer.split(readyPromptRex);
outputBuffer = segments.pop();
Copy link
Member

Choose a reason for hiding this comment

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

Truffle's test data won't fail on this, but as a general purpose solution this might not work. For example:

`this is a line
truffle(develop)>
this is a line
truffle(develop)>
this is a line
`

You'll skip one of the truffle(develop)> lines, whereas I think you previous version would have properly handled it.

cds-amal and others added 5 commits September 6, 2022 19:48
Currently runInREPL sends all queued commands to the child process on
the first detected REPL prompt and closes the input stream on the
second REPL prompt detection which terminates the child process (bad).

This PR reworks the logic to send one queued command per REPL prompt.
Co-authored-by: David Murdoch <187813+davidmurdoch@users.noreply.github.com>
@cds-amal cds-amal merged commit 3474e33 into develop Sep 7, 2022
@cds-amal cds-amal deleted the wait-for-it branch September 7, 2022 21:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants