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

Introduce command line options --norepl and --nointeract, remove IsLIBGAP constant #2723

Merged
merged 2 commits into from
Aug 24, 2018

Conversation

markuspf
Copy link
Member

This is useful for "libgap" as introduced in #2702 and possibly using GAP in "batch mode" as one can leave out the QUIT_GAP(0); in GAP scripts.

This makes the IsLIBGAP constant obsolete, so we remove it as well.

@markuspf markuspf added kind: new feature release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 21, 2018
@markuspf markuspf changed the title Introduce command line option to disable shell, remove IsLIBGAP constant Introduce command line option --noshell, remove IsLIBGAP constant Aug 21, 2018
@markuspf
Copy link
Member Author

Could be two PRs but the two commits are trivial enough I think.

@ChrisJefferson
Copy link
Contributor

Seems sensible, it's a thing I've wanted before actually, when running GAP from a script.

Super picky, should it be --norepl (or something like that) , just because you can still drop into the break loop and do things in there, which feels a bit like the shell, or should it automatically also enable --nobreakloop (do you want to always pair noshell with nobreakloop for your purposes?)

@ChrisJefferson
Copy link
Contributor

Note, I'm happy for the option, and it's name, to be exactly what it is now, just wanted to see if it had been thought about :)

@markuspf
Copy link
Member Author

I considered calling it --norepl, but the REPL is called Shell in GAP as far as I know;

You're right about the breakloop (when I tried it out here just now I ended up in the breakloop); For "libgap" purposes one would never want a break loop as far as I am concerned.

Not sure whether one wants --nointeractive or --batch that just disables both?

@codecov
Copy link

codecov bot commented Aug 21, 2018

Codecov Report

Merging #2723 into master will decrease coverage by <.01%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master    #2723      +/-   ##
==========================================
- Coverage   75.78%   75.78%   -0.01%     
==========================================
  Files         478      478              
  Lines      241434   241438       +4     
==========================================
+ Hits       182974   182975       +1     
- Misses      58460    58463       +3
Impacted Files Coverage Δ
lib/init.g 71.76% <ø> (ø) ⬆️
lib/system.g 66.66% <50%> (-0.32%) ⬇️
src/iostream.c 62.35% <0%> (-1.15%) ⬇️
src/stats.c 94.69% <0%> (-0.2%) ⬇️
hpcgap/lib/hpc/stdtasks.g 64.7% <0%> (+0.76%) ⬆️

@markuspf
Copy link
Member Author

@ChrisJefferson I introduced --norepl and --nointeract; --norepl just disables the repl, but still gives you a break loop, whereas --nointeract disables all interaction (so it sets --norepl and --nobreakloop`).

@markuspf markuspf changed the title Introduce command line option --noshell, remove IsLIBGAP constant Introduce command line options --norepl and --nointeract, remove IsLIBGAP constant Aug 23, 2018
@markuspf
Copy link
Member Author

If anyone (maybe in particular @stevelinton @fingolfin @ChrisJefferson @sebasguts) has any opinions on the naming bikeshed (on these command line options), I am all ears.

@ChrisJefferson
Copy link
Contributor

Those are fine names for me

lib/system.g Outdated
rec( long := "enableMemCheck", default := false)
rec( long := "enableMemCheck", default := false),
rec( long := "norepl", default := false,
help := [ "Disable the GAP REPL" ] ),
Copy link
Member

Choose a reason for hiding this comment

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

There are people who don't know what a REPL is. There is also no help under ?REPL. Of course one can google, but I'd prefer if we did not force people to do that. Can we perhaps expand this, e.g. by writing "Disable the GAP read-eval-print loop (REPL)" or so?

Markus Pfeiffer added 2 commits August 24, 2018 15:54
…AP shell

* --norepl just disables the read-evaluate-print-loop, so GAP can still
  go into a break loop

* --nointeract disables both the repl and the break loop
The presence of the `--norepl` command line option makes the `IsLIBGAP`
constant obsolete, so we remove it.
@markuspf
Copy link
Member Author

@fingolfin updated the help text, and rebased on current master.

I mentioned in the discussion before that "REPL" might not be as well-known a phrase as one might think, but also agreed with @ChrisJefferson that --noshell might be misunderstood to mean no interaction at all.

@markuspf markuspf merged commit aefba81 into gap-system:master Aug 24, 2018
@fingolfin fingolfin added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: new feature release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants