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

Query interpreter version to select its arguments/options and even some specific configuration #172

Closed
eldipa opened this issue May 20, 2021 · 0 comments · Fixed by #173
Closed
Labels
enhancement something nice to have but it is not neither critical nor urgent
Milestone

Comments

@eldipa
Copy link
Collaborator

eldipa commented May 20, 2021

Describe the feature you'd like
byexample should query the interpreter and get its version. The idea is that an interpreter may deprecate some flags or add new ones in future releases.

byexample has a single set of flags and configuration per interpreter. Detecting which version is, it will allow to have a flags/options/configuration per version.

A concrete case is irb 1.2.1 (ruby). In older versions, irb works fine with the --readline flag but since 1.2.1 it breaks (#162 ).

The issue can be fixed adding two more options --nomultiline and --nocolorize. But these are incompatible with older version of irb.

This is a concrete example where we need to have a specific config per interpreter version.

Additional context (optional)

Performance

Querying means that byexample will have to spawn an interpreter. It has a non-trivial runtime cost so we should cache it once per byexample run.

We could use subprocess.check_output instead of pexpect because we don't need any fancy with terminals. check_output shell parameter must be False to ensure that the command will not be interpreted by any shell like in the case of pexpect.

Compatibility

Spawning an interpreter it means that we have to honor the shebang configuration. Currently this is %e %p %a.

We could replace the interpreter's default value for %a by --version or something similar. There could be a problem if the user changes the shebang and it does not use %a so no --version flag will be put and the run will fail.

We could force the --version flag after the shebang but it will break in other cases (probably).

Even if %a is used, the user may had changed the interpreter binary (%p) and now the --version is not the correct flag for that binary or if it is, it may not output the same as the original binary and the parsing of the version string could fail.

Given all of this I'm inclined to go for the "replace %a" strategy and make the "query interpreter version" a "best effort".

@eldipa eldipa added the enhancement something nice to have but it is not neither critical nor urgent label May 20, 2021
@eldipa eldipa added this to the 10.1.0 milestone May 20, 2021
eldipa added a commit that referenced this issue May 23, 2021
We found that in irb >= 1.2.0 the interpreter outputs nonsense.

The use of `-f`, `--nomultiline`, `--nocolorize` and `--noreadline`
fixes the problem (suggested by @fnordfish).

Because `--nomultiline` and `--nocolorize` are not valid options for irb
< 1.2.0, we require to detect the version of irb to properly choose the
correct options.

Closes #162 and closes #172
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement something nice to have but it is not neither critical nor urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant