On Mon, Feb 3, 2014 at 6:42 AM, Jean-Marc Lasgouttes <lasgout...@lyx.org> wrote:
> 31/01/2014 07:02, Scott Kostyshak:
>>
>> On Wed, Jan 29, 2014 at 3:37 AM, Scott Kostyshak <skost...@lyx.org> wrote:
>>>
>>> Ah, right. OK then, I will make a patch for using libScriptSearch in
>>> SystemCall::startscript().
>>
>>
>> Attached is a patch. Note that I also made the change in
>> ForkedCall::startScript. I'm not confident in this patch and have only
>> tested it briefly.
>
>
> Hello Scott,
>
> The patch looks reasonable and I have only a couple of remarks
>
> * the description of $$s in start[Ss]cript shall be made more precise. The
> best is probably to mention that libScriptSearch is applied to the command.
Done in the first patch attached. I did not explain what
libScriptSearch does though (let me know if I should).
> * the special code to handle python in Systemcall::startscript is probably
> also of interest to ForkedCall::startScript. It may be good to factor it out
> somewhere, or even to include it in libScriptSearch (which should then be
> renamed).
Good idea. I put the quote-related python code in libScriptSearch in
the first patch.
I put the other python code (substitutions) in libScriptSearch in the
second patch. Do you have an idea for the new name of libScriptSearch
once this is done? I guess the unifying theme is substituting the
command. 'commandSub'? Or perhaps 'commandPrep' (preparing the command
to be run) ? I used 'commandPrep' in the patch.
> In any case, this is probably not usable to 2.1 (maybe backported later). I
> understand this was the precise reason why you wanted to do that. In this
> case, a solution maybe to, in parallel, reserrect your old simple patch to
> add libScriptSearch here and there.
Agreed that it's not for 2.1. Since no one has reported this before
and I guess it's been around for a while, I'm not worried. For 2.1 I'm
fine with my old simple patch or with modifying Customization.lyx to
mention this temporary bug (otherwise the example there does not
work). Which do you prefer?
Scott
From fe7158d0bc92c242c4ecddce764be921204fcbde Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Thu, 30 Jan 2014 22:59:33 -0500
Subject: [PATCH 1/2] Centralize replacement of "$$s"
Now the replacement is done in startScript(). In addition to making
the code cleaner and more consistent, this commit fixes a bug where
"$$s" was not replaced when "latex=" was specified in the extra flags
of a converter.
---
src/Converter.cpp | 4 +---
src/Format.cpp | 2 +-
src/Mover.cpp | 2 +-
src/graphics/GraphicsConverter.cpp | 4 +---
src/graphics/PreviewLoader.cpp | 2 +-
src/support/ForkedCalls.cpp | 4 ++--
src/support/ForkedCalls.h | 3 ++-
src/support/Systemcall.cpp | 12 +++++++-----
src/support/Systemcall.h | 8 +++++---
src/support/filetools.cpp | 7 ++++++-
src/support/filetools.h | 3 +--
11 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/src/Converter.cpp b/src/Converter.cpp
index a27d742..8085b15 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -450,7 +450,6 @@ bool Converters::convert(Buffer const * buffer,
command = subst(command, token_orig_path,
quoteName(onlyPath(orig_from.absFileName())));
command = subst(command, token_orig_from,
quoteName(onlyFileName(orig_from.absFileName())));
command = subst(command, token_encoding, buffer ?
buffer->params().encoding().iconvName() : string());
- command = libScriptSearch(command);
if (!conv.parselog.empty())
command += " 2> " + quoteName(infile2 + ".out");
@@ -494,8 +493,7 @@ bool Converters::convert(Buffer const * buffer,
if (!conv.parselog.empty()) {
string const logfile = infile2 +
".log";
- string const script =
libScriptSearch(conv.parselog);
- string const command2 = script +
+ string const command2 = conv.parselog +
" < " + quoteName(infile2 +
".out") +
" > " + quoteName(logfile);
one.startscript(Systemcall::Wait,
diff --git a/src/Format.cpp b/src/Format.cpp
index ff03ab5..ef6e679 100644
--- a/src/Format.cpp
+++ b/src/Format.cpp
@@ -637,7 +637,7 @@ bool Formats::view(Buffer const & buffer, FileName const &
filename,
}
}
- string command = libScriptSearch(format->viewer());
+ string command = format->viewer();
if (format_name == "dvi" &&
!lyxrc.view_dvi_paper_option.empty()) {
diff --git a/src/Mover.cpp b/src/Mover.cpp
index 13ee2c8..a901d98 100644
--- a/src/Mover.cpp
+++ b/src/Mover.cpp
@@ -59,7 +59,7 @@ bool SpecialisedMover::do_copy(FileName const & from,
FileName const & to,
if (command_.empty())
return Mover::do_copy(from, to, latex);
- string command = libScriptSearch(command_);
+ string command = command_;
command = subst(command, "$$i", quoteName(from.toFilesystemEncoding()));
command = subst(command, "$$o", quoteName(to.toFilesystemEncoding()));
command = subst(command, "$$l", quoteName(latex));
diff --git a/src/graphics/GraphicsConverter.cpp
b/src/graphics/GraphicsConverter.cpp
index 207c58c..6fc6b26 100644
--- a/src/graphics/GraphicsConverter.cpp
+++ b/src/graphics/GraphicsConverter.cpp
@@ -322,8 +322,7 @@ static void build_script(string const & from_file,
ostringstream os;
os << os::python() << ' '
- << libScriptSearch("$$s/scripts/convertDefault.py",
- quote_python) << ' ';
+ << "$$s/scripts/convertDefault.py" << ' ';
if (!from_format.empty())
os << strip_digit(from_format) << ':';
// The extra " quotes around infile and outfile are needed
@@ -385,7 +384,6 @@ static void build_script(string const & from_file,
command = subst(command, token_base, "' + '\"' + infile_base +
'\"' + '");
command = subst(command, token_to, "' + '\"' + outfile +
'\"' + '");
command = subst(command, token_todir, "' + '\"' + outdir + '\"'
+ '");
- command = libScriptSearch(command, quote_python);
build_conversion_command(command, script);
}
diff --git a/src/graphics/PreviewLoader.cpp b/src/graphics/PreviewLoader.cpp
index b4d78e0..e03c285 100644
--- a/src/graphics/PreviewLoader.cpp
+++ b/src/graphics/PreviewLoader.cpp
@@ -613,7 +613,7 @@ void PreviewLoader::Impl::startLoading(bool wait)
if (buffer_.params().bufferFormat() == "lilypond-book")
cs << " --lilypond";
- string const command = libScriptSearch(cs.str());
+ string const command = cs.str();
if (wait) {
ForkedCall call(buffer_.filePath());
diff --git a/src/support/ForkedCalls.cpp b/src/support/ForkedCalls.cpp
index d77248e..ed48764 100644
--- a/src/support/ForkedCalls.cpp
+++ b/src/support/ForkedCalls.cpp
@@ -282,7 +282,7 @@ int ForkedCall::startScript(Starttype wait, string const &
what)
return retval_;
}
- command_ = trim(what);
+ command_ = libScriptSearch(trim(what));
signal_.reset();
return run(Wait);
}
@@ -290,7 +290,7 @@ int ForkedCall::startScript(Starttype wait, string const &
what)
int ForkedCall::startScript(string const & what, SignalTypePtr signal)
{
- command_ = trim(what);
+ command_ = libScriptSearch(trim(what));
signal_ = signal;
return run(DontWait);
diff --git a/src/support/ForkedCalls.h b/src/support/ForkedCalls.h
index 529f5c6..1958980 100644
--- a/src/support/ForkedCalls.h
+++ b/src/support/ForkedCalls.h
@@ -158,7 +158,8 @@ public:
/** Start the child process.
*
- * The command "what" is passed to execvp() for execution.
+ * The command "what" is passed to execvp() for execution. "$$s" is
+ * replaced accordingly by libScriptSearch().
*
* There are two startScript commands available. They differ in that
* the second receives a signal that is executed on completion of
diff --git a/src/support/Systemcall.cpp b/src/support/Systemcall.cpp
index 0ab7685..d6e1077 100644
--- a/src/support/Systemcall.cpp
+++ b/src/support/Systemcall.cpp
@@ -105,11 +105,12 @@ int Systemcall::startscript(Starttype how, string const &
what,
{
string const python_call = "python -tt";
string command = to_filesystem8bit(from_utf8(latexEnvCmdPrefix(path)));
+ string what_ss = libScriptSearch(what);
- if (prefixIs(what, python_call))
- command += os::python() + what.substr(python_call.length());
+ if (prefixIs(what_ss, python_call))
+ command += os::python() + what_ss.substr(python_call.length());
else
- command += what;
+ command += what_ss;
if (how == DontWait) {
switch (os::shell()) {
@@ -240,13 +241,14 @@ string const parsecmd(string const & incmd, string &
infile, string & outfile,
int Systemcall::startscript(Starttype how, string const & what,
string const & path, bool process_events)
{
- LYXERR(Debug::INFO,"Running: " << what);
+ string const what_ss = libScriptSearch(what);
+ LYXERR(Debug::INFO,"Running: " << what_ss);
string infile;
string outfile;
string errfile;
QString const cmd = QString::fromLocal8Bit(
- parsecmd(what, infile, outfile, errfile).c_str());
+ parsecmd(what_ss, infile, outfile, errfile).c_str());
SystemcallPrivate d(infile, outfile, errfile);
diff --git a/src/support/Systemcall.h b/src/support/Systemcall.h
index 1d4c753..173387b 100644
--- a/src/support/Systemcall.h
+++ b/src/support/Systemcall.h
@@ -40,10 +40,12 @@ public:
/** Start child process.
* The string "what" contains a commandline with arguments separated
- * by spaces and encoded in the filesystem encoding. The string "path"
+ * by spaces and encoded in the filesystem encoding. "$$s" will be
+ * replaced accordingly by libScriptSearch(). The string "path"
* contains the path to be prepended to the TEXINPUTS environment
- * variable and encoded in utf-8. Unset "process_events" in case
- * UI should be blocked while processing the external command.
+ * variable and encoded in the path to be prepended to the TEXINPUTS
+ * environment variable and utf-8. Unset "process_events" in case UI
+ * should be blocked while processing the external command.
*/
int startscript(Starttype how, std::string const & what,
std::string const & path = empty_string(),
diff --git a/src/support/filetools.cpp b/src/support/filetools.cpp
index 0012481..0de2e13 100644
--- a/src/support/filetools.cpp
+++ b/src/support/filetools.cpp
@@ -337,7 +337,7 @@ FileName const imageLibFileSearch(string & dir, string
const & name,
}
-string const libScriptSearch(string const & command_in, quote_style style)
+string const libScriptSearch(string const & command_in)
{
static string const token_scriptpath = "$$s/";
@@ -361,6 +361,11 @@ string const libScriptSearch(string const & command_in,
quote_style style)
// Replace "$$s/" with ""
command.erase(pos1, 4);
} else {
+ quote_style style = quote_shell;
+ string const python_call = "python -tt";
+ if (prefixIs(command, python_call) || prefixIs(command,
os::python()))
+ style = quote_python;
+
// Replace "$$s/foo/some_script" with "<path to>/some_script".
string::size_type const size_replace = size_script + 4;
command.replace(pos1, size_replace, quoteName(script, style));
diff --git a/src/support/filetools.h b/src/support/filetools.h
index 58d1154..c13b5b3 100644
--- a/src/support/filetools.h
+++ b/src/support/filetools.h
@@ -125,8 +125,7 @@ enum quote_style {
* command will still fail, but the error message will make some sort of
* sense ;-)
*/
-std::string const libScriptSearch(std::string const & command,
- quote_style style = quote_shell);
+std::string const libScriptSearch(std::string const & command);
enum latex_path_extension {
PROTECT_EXTENSION,
--
1.8.3.2
From 11f0f00b4f89425bd94dbc340c5166bf4e5ed78d Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Mon, 3 Feb 2014 10:57:52 -0500
Subject: [PATCH 2/2] Centralize substitution of python commands
The code for detecting python commands and substituting in the
correct prefix is now merged with what used to be libScriptSearch()
and is now renamed to commandPrep().
---
src/support/ForkedCalls.cpp | 4 ++--
src/support/ForkedCalls.h | 2 +-
src/support/Systemcall.cpp | 12 +++---------
src/support/Systemcall.h | 10 +++++-----
src/support/filetools.cpp | 9 ++++++---
src/support/filetools.h | 2 +-
6 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/src/support/ForkedCalls.cpp b/src/support/ForkedCalls.cpp
index ed48764..00dca3e 100644
--- a/src/support/ForkedCalls.cpp
+++ b/src/support/ForkedCalls.cpp
@@ -282,7 +282,7 @@ int ForkedCall::startScript(Starttype wait, string const &
what)
return retval_;
}
- command_ = libScriptSearch(trim(what));
+ command_ = commandPrep(trim(what));
signal_.reset();
return run(Wait);
}
@@ -290,7 +290,7 @@ int ForkedCall::startScript(Starttype wait, string const &
what)
int ForkedCall::startScript(string const & what, SignalTypePtr signal)
{
- command_ = libScriptSearch(trim(what));
+ command_ = commandPrep(trim(what));
signal_ = signal;
return run(DontWait);
diff --git a/src/support/ForkedCalls.h b/src/support/ForkedCalls.h
index 1958980..7729398 100644
--- a/src/support/ForkedCalls.h
+++ b/src/support/ForkedCalls.h
@@ -159,7 +159,7 @@ public:
/** Start the child process.
*
* The command "what" is passed to execvp() for execution. "$$s" is
- * replaced accordingly by libScriptSearch().
+ * replaced accordingly by commandPrep().
*
* There are two startScript commands available. They differ in that
* the second receives a signal that is executed on completion of
diff --git a/src/support/Systemcall.cpp b/src/support/Systemcall.cpp
index d6e1077..bbd451e 100644
--- a/src/support/Systemcall.cpp
+++ b/src/support/Systemcall.cpp
@@ -103,14 +103,8 @@ ProgressInterface* ProgressInterface::instance()
int Systemcall::startscript(Starttype how, string const & what,
std::string const & path, bool /*process_events*/)
{
- string const python_call = "python -tt";
- string command = to_filesystem8bit(from_utf8(latexEnvCmdPrefix(path)));
- string what_ss = libScriptSearch(what);
-
- if (prefixIs(what_ss, python_call))
- command += os::python() + what_ss.substr(python_call.length());
- else
- command += what_ss;
+ string command = to_filesystem8bit(from_utf8(latexEnvCmdPrefix(path)))
+ + commandPrep(what);
if (how == DontWait) {
switch (os::shell()) {
@@ -241,7 +235,7 @@ string const parsecmd(string const & incmd, string &
infile, string & outfile,
int Systemcall::startscript(Starttype how, string const & what,
string const & path, bool process_events)
{
- string const what_ss = libScriptSearch(what);
+ string const what_ss = commandPrep(what);
LYXERR(Debug::INFO,"Running: " << what_ss);
string infile;
diff --git a/src/support/Systemcall.h b/src/support/Systemcall.h
index 173387b..5b0750d 100644
--- a/src/support/Systemcall.h
+++ b/src/support/Systemcall.h
@@ -41,11 +41,11 @@ public:
/** Start child process.
* The string "what" contains a commandline with arguments separated
* by spaces and encoded in the filesystem encoding. "$$s" will be
- * replaced accordingly by libScriptSearch(). The string "path"
- * contains the path to be prepended to the TEXINPUTS environment
- * variable and encoded in the path to be prepended to the TEXINPUTS
- * environment variable and utf-8. Unset "process_events" in case UI
- * should be blocked while processing the external command.
+ * replaced accordingly by commandPrep(). The string "path" contains
+ * the path to be prepended to the TEXINPUTS environment variable and
+ * encoded in the path to be prepended to the TEXINPUTS environment
+ * variable and utf-8. Unset "process_events" in case UI should be
+ * blocked while processing the external command.
*/
int startscript(Starttype how, std::string const & what,
std::string const & path = empty_string(),
diff --git a/src/support/filetools.cpp b/src/support/filetools.cpp
index 0de2e13..4af1a6c 100644
--- a/src/support/filetools.cpp
+++ b/src/support/filetools.cpp
@@ -337,11 +337,15 @@ FileName const imageLibFileSearch(string & dir, string
const & name,
}
-string const libScriptSearch(string const & command_in)
+string const commandPrep(string const & command_in)
{
static string const token_scriptpath = "$$s/";
+ string const python_call = "python -tt";
string command = command_in;
+ if (prefixIs(command_in, python_call))
+ command = os::python() +
command_in.substr(python_call.length());
+
// Find the starting position of "$$s/"
string::size_type const pos1 = command.find(token_scriptpath);
if (pos1 == string::npos)
@@ -362,8 +366,7 @@ string const libScriptSearch(string const & command_in)
command.erase(pos1, 4);
} else {
quote_style style = quote_shell;
- string const python_call = "python -tt";
- if (prefixIs(command, python_call) || prefixIs(command,
os::python()))
+ if (prefixIs(command, os::python()))
style = quote_python;
// Replace "$$s/foo/some_script" with "<path to>/some_script".
diff --git a/src/support/filetools.h b/src/support/filetools.h
index c13b5b3..fbc14f8 100644
--- a/src/support/filetools.h
+++ b/src/support/filetools.h
@@ -125,7 +125,7 @@ enum quote_style {
* command will still fail, but the error message will make some sort of
* sense ;-)
*/
-std::string const libScriptSearch(std::string const & command);
+std::string const commandPrep(std::string const & command);
enum latex_path_extension {
PROTECT_EXTENSION,
--
1.8.3.2
