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

Move argv/1 to library(os) #2263

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Move argv/1 to library(os) #2263

merged 1 commit into from
Jan 2, 2024

Conversation

aarroyoc
Copy link
Contributor

@aarroyoc aarroyoc commented Jan 2, 2024

By moving argv/1 to library(os), this predicate can be found and documented just like any other predicate. In addition, special code to handle the arguments in the top-level has been simplified.

@mthom mthom merged commit 5c89029 into mthom:master Jan 2, 2024
13 checks passed
@aarroyoc aarroyoc deleted the docs-toplevel branch January 2, 2024 16:28
@@ -1,5 +1,4 @@
:- module('$toplevel', [argv/1,
copy_term/3]).
:- module('$toplevel', [copy_term/3, started/0]).
Copy link
Contributor

@triska triska Jan 2, 2024

Choose a reason for hiding this comment

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

why is started/0 exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

loader.pl uses it and I thought of leaving it as explicit as before

Copy link
Contributor

Choose a reason for hiding this comment

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

I think loader.pl uses the module-qualified version, so there seems no need to export such a "normal" (i.e., prone to naming conflicts, even though no program likely needs it) name?

@Skgland
Copy link
Contributor

Skgland commented Jan 2, 2024

I think this might be wrong in case the list of arguments contains more than one instance of --.

E.g.

$ cargo run --release -- -f -- -t -- hello
    Finished release [optimized] target(s) in 0.12s
     Running `target/release/scryer-prolog -f -- -t -- hello`
?- os:argv(V).
   V = ["-t","--","hello"].
?- os:argv(["-t"|_]).
   true.
?- os:argv(["--"|_]).
   false.
?- os:argv(["hello"|_]).
   true.  %% unexpected
?- 

Skgland added a commit to Skgland/scryer-prolog that referenced this pull request Jan 2, 2024
@@ -23,6 +23,6 @@ fn main() -> std::process::ExitCode {

runtime.block_on(async move {
let mut wam = machine::Machine::new(Default::default());
wam.run_top_level(atom!("$toplevel"), (atom!("$repl"), 1))
wam.run_module_predicate(atom!("$toplevel"), (atom!("$repl"), 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a great change, and paves the way to flexibly specifying the goal that should run as toplevel, for example with -t Goal on the command line. We could then run scripts with:

$ scryer-prolog -t halt -g Goal

and ensure that Scryer Prolog halts after goal succeeds or fails or raises an exception!

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