Skip to content

Commit

Permalink
Add Exec2, the "better version of Exec"
Browse files Browse the repository at this point in the history
Add a new function `Exec2` which is as easy to use as `Exec` but
much more powerful and less easy to misuse:

- don't pass everything to a shell, thus avoiding compatibility
  issues due to different shells on different systems (this also
  has one downside: you can't use redirects like ">/dev/null")
- pass arguments as separate strings, thus avoiding any issues with
  quoting quotes, quoting spaces, etc.
- return the exit code of the executed command so that one can
  determine its success or failure; the return value is actually
  a record to allow returning other data, such as the programs output
- make it very easy to override the input and output streams
- by default, pass no input to the program (instead of passing
  anything the user might type, as `Exec` does); this can be
  changed by adding an input stream to the argument list
- by default, capture any output of the program into a string
  and return it, instead of just printing the output to the
  console; this can be changed by adding an output stream to
  the argument list
  • Loading branch information
fingolfin committed Feb 12, 2024
1 parent deada08 commit 398fa01
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 29 deletions.
53 changes: 29 additions & 24 deletions lib/helpview.gi
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ if ARCH_IS_WINDOWS() then
winfilename:=MakeExternalFilename( SplitString( filename, "#" )[1] );
fi;
Print( "Opening help page ", winfilename, " in default windows browser ... \c" );
Exec( Concatenation("start ", winfilename ) );
Exec2( "start", winfilename );

Check warning on line 83 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L83

Added line #L83 was not covered by tests
Print( "done! \n" );
end
);
Expand Down Expand Up @@ -138,7 +138,7 @@ elif ARCH_IS_MAC_OS_X() then
fi;
file := file.file;
fi;
Exec(Concatenation("open -a Preview ", file));
Exec2("open", "-a", "Preview", file);

Check warning on line 141 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L141

Added line #L141 was not covered by tests
Print("# see page ", page, " in the Preview window.\n");
end
);
Expand All @@ -155,7 +155,7 @@ elif ARCH_IS_MAC_OS_X() then
fi;
file := file.file;
fi;
Exec(Concatenation("open -a \"Adobe Reader\" ", file));
Exec2("open", "-a", "Adobe Reader", file);

Check warning on line 158 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L158

Added line #L158 was not covered by tests
Print("# see page ", page, " in the Adobe Reader window.\n");
end
);
Expand All @@ -172,7 +172,7 @@ elif ARCH_IS_MAC_OS_X() then
fi;
file := file.file;
fi;
Exec(Concatenation("open ", file));
Exec2("open ", file);

Check warning on line 175 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L175

Added line #L175 was not covered by tests
Print("# see page ", page, " in the pdf viewer window.\n");
end
);
Expand All @@ -187,16 +187,15 @@ elif ARCH_IS_MAC_OS_X() then
fi;
file := file.file;
fi;
Exec( Concatenation(
"osascript <<ENDSCRIPT\n",
"tell application \"Skim\"\n",
"activate\n",
"open \"", file, "\"\n",
"set theDoc to document of front window\n",
"go theDoc to page ",String(page)," of theDoc\n",
"end tell\n",
"ENDSCRIPT\n" ) );
return;
Exec2("osascript",
InputTextString(Concatenation("""

Check warning on line 191 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L190-L191

Added lines #L190 - L191 were not covered by tests
tell application "Skim"
activate
open """, ViewString(file), """

Check warning on line 194 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L194

Added line #L194 was not covered by tests
set theDoc to document of front window
go theDoc to page """,String(page),""" of theDoc

Check warning on line 196 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L196

Added line #L196 was not covered by tests
end tell
""")));

Check warning on line 198 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L198

Added line #L198 was not covered by tests
end
);

Expand All @@ -207,17 +206,20 @@ else # UNIX but not macOS
HELP_VIEWER_INFO.browser := rec(
type := "url",
show := function( url )
local str;
str := "";
Exec2("wslpath", "-a", "-w", url, OutputTextString(str, false));

Check warning on line 211 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L210-L211

Added lines #L210 - L211 were not covered by tests
# Ignoring part of the URL after '#' since we are unable
# to navigate to the precise location on Windows
url := SplitString( url, "#" )[1];
Exec(Concatenation("explorer.exe \"$(wslpath -a -w \"",url, "\")\""));
Exec2("explorer.exe", str);

Check warning on line 215 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L215

Added line #L215 was not covered by tests
end
);

HELP_VIEWER_INFO.("pdf viewer") := rec(
type := "pdf",
show := function(file)
local page;
local page, str;
# unfortunately one cannot (yet?) give a start page to windows
page := 1;
if IsRecord(file) then
Expand All @@ -226,7 +228,10 @@ else # UNIX but not macOS
fi;
file := file.file;
fi;
Exec(Concatenation("explorer.exe \"$(wslpath -a -w \"",file, "\")\""));

str := "";
Exec2("wslpath", "-a", "-w", url, OutputTextString(str, false));
Exec2("explorer.exe", str);

Check warning on line 234 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L232-L234

Added lines #L232 - L234 were not covered by tests
Print("# see page ", page, " in PDF.\n");
end
);
Expand All @@ -235,15 +240,15 @@ else # UNIX but not macOS
HELP_VIEWER_INFO.netscape := rec(
type := "url",
show := function(url)
Exec(Concatenation("netscape -remote \"openURL(file:", url, ")\""));
Exec2("netscape", "-remote", Concatenation("openURL(file:", url, ")"));

Check warning on line 243 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L243

Added line #L243 was not covered by tests
end
);

# html version with mozilla
HELP_VIEWER_INFO.mozilla := rec(
type := "url",
show := function(url)
Exec(Concatenation("mozilla -remote \"openURL(file:", url, ")\""));
Exec2("mozilla", "-remote", Concatenation("openURL(file:", url, ")"));

Check warning on line 251 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L251

Added line #L251 was not covered by tests
end
);

Expand Down Expand Up @@ -276,36 +281,36 @@ else # UNIX but not macOS
HELP_VIEWER_INFO.lynx := rec(
type := "url",
show := function(url)
Exec(Concatenation("lynx \"", url, "\""));
Exec2("lynx", url);

Check warning on line 284 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L284

Added line #L284 was not covered by tests
end
);

# html version with w3m
HELP_VIEWER_INFO.w3m := rec(
type := "url",
show := function(url)
Exec(Concatenation("w3m \"", url, "\""));
Exec2("w3m", url);

Check warning on line 292 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L292

Added line #L292 was not covered by tests
end
);

HELP_VIEWER_INFO.elinks := rec(
type := "url",
show := function(url)
Exec(Concatenation("elinks \"", url, "\""));
Exec2("elinks", url);

Check warning on line 299 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L299

Added line #L299 was not covered by tests
end
);

HELP_VIEWER_INFO.links2ng := rec(
type := "url",
show := function(url)
Exec(Concatenation("links2 \"", url, "\""));
Exec2("links2", url);

Check warning on line 306 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L306

Added line #L306 was not covered by tests
end
);

HELP_VIEWER_INFO.links2 := rec(
type := "url",
show := function(url)
Exec(Concatenation("links2 -g \"", url, "\""));
Exec2("links2", "-g", url);

Check warning on line 313 in lib/helpview.gi

View check run for this annotation

Codecov / codecov/patch

lib/helpview.gi#L313

Added line #L313 was not covered by tests
end
);
fi;
Expand Down
2 changes: 2 additions & 0 deletions lib/process.gd
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,5 @@ DeclareOperation( "Process",
## <#/GAPDoc>
##
DeclareGlobalFunction( "Exec" );

DeclareGlobalName( "Exec2" );
74 changes: 74 additions & 0 deletions lib/process.gi
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,77 @@ InstallGlobalFunction( Exec, function( arg )
Process( dir, shell, InputTextUser(), OutputTextUser(), [ cs, cmd ] );

end );

# TODO: document this, come up with a better name, write some tests...
BindGlobal( "Exec2", function( arg )
local args, result, a, input, output, dir, cmd;

args := [];
result := rec();

Check warning on line 270 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L269-L270

Added lines #L269 - L270 were not covered by tests

# parse the inputs
for a in arg do
if IsDirectory(a) then
if IsBound(dir) then
Error("must specify at most one working directory");
fi;
dir := a;
elif IsInputStream(a) then
if IsBound(input) then
Error("must specify at most one input stream");
fi;
input := a;
elif IsOutputStream(a) then
if IsBound(output) then
Error("must specify at most one output stream");
fi;
output := a;
elif IsString(a) then
ConvertToStringRep(a);
if not IsBound(cmd) then
cmd := a;

Check warning on line 292 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L274-L292

Added lines #L274 - L292 were not covered by tests
else
Add(args, a);
fi;
else
Error("unsupported argument type");
fi;
od;

Check warning on line 299 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L294-L299

Added lines #L294 - L299 were not covered by tests

if not IsBound(cmd) then
Error("must specify a command to execute");
fi;

Check warning on line 303 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L301-L303

Added lines #L301 - L303 were not covered by tests

# determine full executable path if it is not already a path
if not '/' in cmd then
a := Filename( DirectoriesSystemPrograms(), cmd );
if a = fail and ARCH_IS_WINDOWS() then
a := Filename( DirectoriesSystemPrograms(), Concatenation( cmd, ".exe" ) );
fi;
if a = fail then
Error("could not locate executable for '", cmd, "'");
fi;
cmd := a;
fi;

Check warning on line 315 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L306-L315

Added lines #L306 - L315 were not covered by tests

# set default working directory if necessary
if not IsBound(dir) then
dir := DirectoryCurrent();
fi;

Check warning on line 320 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L318-L320

Added lines #L318 - L320 were not covered by tests

# if no input stream was specified, pass no input to the command
if not IsBound(input) then
input := InputTextNone();
fi;

Check warning on line 325 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L323-L325

Added lines #L323 - L325 were not covered by tests

# if no output stream was specified, put output into the returned record
if not IsBound(output) then
result.output := "";
output := OutputTextString(result.output, false);
fi;

Check warning on line 331 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L328-L331

Added lines #L328 - L331 were not covered by tests

# execute the command
result.status := Process( dir, cmd, input, output, args );

Check warning on line 334 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L334

Added line #L334 was not covered by tests

return result;
end );

Check warning on line 337 in lib/process.gi

View check run for this annotation

Codecov / codecov/patch

lib/process.gi#L336-L337

Added lines #L336 - L337 were not covered by tests
11 changes: 6 additions & 5 deletions lib/streams.gi
Original file line number Diff line number Diff line change
Expand Up @@ -1311,19 +1311,20 @@ InstallGlobalFunction( InputFromUser,
InstallGlobalFunction( OpenExternal, function(filename)
local file;
if ARCH_IS_MAC_OS_X() then
Exec(Concatenation("open \"",filename,"\""));
Exec2("open", filename);

Check warning on line 1314 in lib/streams.gi

View check run for this annotation

Codecov / codecov/patch

lib/streams.gi#L1314

Added line #L1314 was not covered by tests
elif ARCH_IS_WINDOWS() then
Exec(Concatenation("cmd /c start \"",filename,"\""));
Exec2("cmd", "/c", "start", filename);

Check warning on line 1316 in lib/streams.gi

View check run for this annotation

Codecov / codecov/patch

lib/streams.gi#L1316

Added line #L1316 was not covered by tests
elif ARCH_IS_WSL() then
# If users pass a URL, make sure if does not get mangled.
if ForAny(["https://", "http://"], {pre} -> StartsWith(filename, pre)) then
file := filename;
else
file := Concatenation("$(wslpath -a -w \"",filename,"\")");
file := "";
Exec2("wslpath", "-a", "-w", filename, OutputTextString(file, false));

Check warning on line 1323 in lib/streams.gi

View check run for this annotation

Codecov / codecov/patch

lib/streams.gi#L1322-L1323

Added lines #L1322 - L1323 were not covered by tests
fi;
Exec(Concatenation("explorer.exe \"", file, "\""));
Exec2("explorer.exe", file);

Check warning on line 1325 in lib/streams.gi

View check run for this annotation

Codecov / codecov/patch

lib/streams.gi#L1325

Added line #L1325 was not covered by tests
else
Exec(Concatenation("xdg-open \"",filename,"\""));
Exec2("xdg-open", filename);

Check warning on line 1327 in lib/streams.gi

View check run for this annotation

Codecov / codecov/patch

lib/streams.gi#L1327

Added line #L1327 was not covered by tests
fi;
end );

Expand Down

0 comments on commit 398fa01

Please sign in to comment.