Am 01.11.2010 um 23:42 schrieb Pavel Sanda:

> Stephan Witt wrote:
>> Sorry, I went out and could not answer :-)
>> 
>> 1) I have a patch at hand to add the ability to collect stderr in system call
>> and to simplify the stdout collection too. This patch doesn't change the
>> current interface but adds a second one. VCS is more clean with that.
>> The need to add ">" redirection and parse it later to split it vanishes.
> 
> we are entering new flame even before posting this patch...
> to add new interface for each of the backend is not a good thing.

I meant a new interface for SystemCall with stdout and stderr parameters.

> all the code should use the same infrastructure and workarounding my request
> not to refactor vcs code base by implementing new lyxvc_for_the_sake_of_cvs
> is not the way i would like to continue ;)

Of course. That I didn't want.

> if you have the patch already feel free to post it, but...

I'll attach it - so you get the idea.

I want to
1) catch the output of failing system calls to display them in error message 
dialog
2) eventually be able to pass it up to caller for parsing it
3) simplify the tmpfile handling

> 
>> 2) I want to complete the CVS implementation. 
>> Add the missing buffer-info things. Add the diff to previous version.
>> 
>> 3) Want the readFile() issue fixed.
> 
> is this beta1 critical stuf?

After my last commit in readFile() it's "only" the issue with file_found_hook 
being called with the wrong file name in case of autosave and emergency.
I'd like to fix it before (2) or maybe Vincent does it... For users of VCS I'd 
rate it as a serious problem - but not critical.

Stephan

Index: src/support/Systemcall.cpp
===================================================================
--- src/support/Systemcall.cpp  (Revision 35961)
+++ src/support/Systemcall.cpp  (Arbeitskopie)
@@ -207,14 +207,21 @@
 int Systemcall::startscript(Starttype how, string const & what, bool 
process_events)
 {
        string outfile;
+       string errfile;
        QString cmd = toqstr(parsecmd(what, outfile));
+       return startscript(how, what, outfile, errfile, process_events);
+}
 
-       SystemcallPrivate d(outfile);
 
+int Systemcall::startscript(Starttype how, string const & what,
+                                                       std::string const & 
outfile, std::string const & errfile, 
+                                                       bool process_events)
+{
+       SystemcallPrivate d(outfile, errfile);
 
-       d.startProcess(cmd);
+       d.startProcess(toqstr(what));
        if (!d.waitWhile(SystemcallPrivate::Starting, process_events, -1)) {
-               LYXERR0("Systemcall: '" << cmd << "' did not start!");
+               LYXERR0("Systemcall: '" << what << "' did not start!");
                LYXERR0("error " << d.errorMessage());
                return 10;
        }
@@ -226,7 +233,7 @@
        }
 
        if (!d.waitWhile(SystemcallPrivate::Running, process_events, 180000)) {
-               LYXERR0("Systemcall: '" << cmd << "' did not finished!");
+               LYXERR0("Systemcall: '" << what << "' did not finished!");
                LYXERR0("error " << d.errorMessage());
                LYXERR0("status " << d.exitStatusMessage());
                return 20;
@@ -234,18 +241,18 @@
 
        int const exit_code = d.exitCode();
        if (exit_code) {
-               LYXERR0("Systemcall: '" << cmd << "' finished with exit code " 
<< exit_code);
+               LYXERR0("Systemcall: '" << what << "' finished with exit code " 
<< exit_code);
        }
 
        return exit_code;
 }
 
 
-SystemcallPrivate::SystemcallPrivate(const std::string& of) :
+SystemcallPrivate::SystemcallPrivate(const std::string& out_file) :
                                 process_(new QProcess), 
                                 out_index_(0),
                                 err_index_(0),
-                                out_file_(of), 
+                                out_file_(out_file), 
                                 process_events_(false)
 {
        if (!out_file_.empty()) {
@@ -262,7 +269,33 @@
 }
 
 
+SystemcallPrivate::SystemcallPrivate(const std::string& out_file, const 
std::string& err_file) :
+               process_(new QProcess), 
+               out_index_(0),
+               err_index_(0),
+               out_file_(out_file), 
+               err_file_(err_file), 
+               process_events_(false)
+{
+       if (!out_file_.empty()) {
+               // Check whether we have to simply throw away the output.
+               if (out_file_ != os::nulldev())
+                       process_->setStandardOutputFile(toqstr(out_file_));
+       }
+       if (!err_file_.empty()) {
+               // Check whether we have to simply throw away the error output.
+               if (err_file_ != os::nulldev())
+                       process_->setStandardErrorFile(toqstr(err_file_));
+       }
+       
+       connect(process_, SIGNAL(readyReadStandardOutput()), SLOT(stdOut()));
+       connect(process_, SIGNAL(readyReadStandardError()), SLOT(stdErr()));
+       connect(process_, SIGNAL(error(QProcess::ProcessError)), 
SLOT(processError(QProcess::ProcessError)));
+       connect(process_, SIGNAL(started()), this, SLOT(processStarted()));
+       connect(process_, SIGNAL(finished(int, QProcess::ExitStatus)), 
SLOT(processFinished(int, QProcess::ExitStatus)));
+}
 
+
 void SystemcallPrivate::startProcess(const QString& cmd)
 {
        cmd_ = cmd;
Index: src/support/Systemcall.h
===================================================================
--- src/support/Systemcall.h    (Revision 35961)
+++ src/support/Systemcall.h    (Arbeitskopie)
@@ -44,6 +44,9 @@
         *  processing the external command.
         */
        int startscript(Starttype how, std::string const & what, bool 
process_events = false);
+       int startscript(Starttype how, std::string const & what,
+                                       std::string const & outfile, 
std::string const & errfile, 
+                                       bool process_events = false);
 };
 
 } // namespace support
Index: src/support/SystemcallPrivate.h
===================================================================
--- src/support/SystemcallPrivate.h     (Revision 35961)
+++ src/support/SystemcallPrivate.h     (Arbeitskopie)
@@ -33,6 +33,7 @@
 
 public:
        SystemcallPrivate(std::string const & outfile);
+       SystemcallPrivate(std::string const & outfile, std::string const & 
errfile);
        ~SystemcallPrivate();
 
        enum State {
@@ -74,6 +75,8 @@
        size_t err_index_;
        ///
        std::string out_file_;
+       ///
+       std::string err_file_;
 
        /// Size of buffers.
        static size_t const buffer_size_ = 200;
Index: src/VCBackend.h
===================================================================
--- src/VCBackend.h     (Revision 35961)
+++ src/VCBackend.h     (Arbeitskopie)
@@ -101,7 +101,20 @@
         */
        static int doVCCommandCall(std::string const & cmd, support::FileName 
const & path);
 
+       // variation with output collector
+       int doVCCommand(std::string const & cmd, support::FileName const & path,
+                       support::FileName const & output, bool reportError = 
true);
        /**
+        * doVCCommandCall - call out to the version control utility
+        * @param cmd the command to execute
+        * @param path the path from which to execute
+        * @param output the path where to store output
+        * @return exit status
+        */
+       static int doVCCommandCall(std::string const & cmd, support::FileName 
const & path,
+               support::FileName const & output, support::FileName const & 
error);
+
+       /**
         * The master VC file. For RCS this is *,v or RCS/ *,v. master should
         * have full path.
         */
Index: src/VCBackend.cpp
===================================================================
--- src/VCBackend.cpp   (Revision 35961)
+++ src/VCBackend.cpp   (Arbeitskopie)
@@ -49,18 +49,41 @@
 
 int VCS::doVCCommand(string const & cmd, FileName const & path, bool 
reportError)
 {
+       return doVCCommand(cmd, path, FileName(), reportError);
+}
+
+
+int VCS::doVCCommandCall(string const & cmd, FileName const & path, FileName 
const & output, FileName const & error)
+{
+       LYXERR(Debug::LYXVC, "doVCCommandCall: " << cmd);
+       Systemcall one;
+       support::PathChanger p(path);
+       return one.startscript(Systemcall::Wait, cmd, output.absFileName(), 
error.absFileName(), false);
+}
+
+
+int VCS::doVCCommand(string const & cmd, FileName const & path, FileName const 
& output, bool reportError)
+{
        if (owner_)
                owner_->setBusy(true);
 
-       int const ret = doVCCommandCall(cmd, path);
+       FileName error = FileName::tempName("lyxvcerr");
+       int const ret = doVCCommandCall(cmd, path, output.empty() ? error : 
output, error);
 
        if (owner_)
                owner_->setBusy(false);
-       if (ret && reportError)
+       if (ret && reportError) {
+               docstring msg = error.fileContents("UTF-8");
+               if (msg.length() > 300) {
+                       msg = msg.substr(0,300) + "...";
+               }
                frontend::Alert::error(_("Revision control error."),
                        bformat(_("Some problem occured while running the 
command:\n"
-                                 "'%1$s'."),
-                       from_utf8(cmd)));
+                                         "'%1$s'.\n"
+                                         "%2$s"),
+                                       from_utf8(cmd), msg));
+       }
+       error.removeFile();
        return ret;
 }
 
@@ -277,9 +300,8 @@
 
 void RCS::getLog(FileName const & tmpf)
 {
-       doVCCommand("rlog " + quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+       doVCCommand("rlog " + quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
 }
 
 
@@ -336,9 +358,8 @@
        }
 
        doVCCommand("co -p" + rev + " "
-                     + quoteName(onlyFileName(owner_->absFileName()))
-                     + " > " + quoteName(tmpf.toFilesystemEncoding()),
-               FileName(owner_->filePath()));
+               + quoteName(onlyFileName(owner_->absFileName())),
+               FileName(owner_->filePath()), tmpf);
        if (tmpf.isFileEmpty())
                return false;
 
@@ -483,9 +504,8 @@
                return StatusError;
        }
 
-       if (doVCCommand("cvs status " + getTarget(File)
-               + " > " + quoteName(tmpf.toFilesystemEncoding()),
-               FileName(owner_->filePath()))) {
+       if (doVCCommand("cvs status " + getTarget(File),
+               FileName(owner_->filePath()), tmpf)) {
                tmpf.removeFile();
                return StatusError;
        }
@@ -525,9 +545,8 @@
 
 void CVS::getDiff(OperationMode opmode, FileName const & tmpf)
 {
-       doVCCommand("cvs diff " + getTarget(opmode)
-               + " > " + quoteName(tmpf.toFilesystemEncoding()),
-               FileName(owner_->filePath()), false);
+       doVCCommand("cvs diff " + getTarget(opmode),
+               FileName(owner_->filePath()), tmpf, false);
 }
 
 
@@ -549,12 +568,9 @@
 
 int CVS::update(OperationMode opmode, FileName const & tmpf)
 {
-       string const redirection = tmpf.empty() ? ""
-               : " > " + quoteName(tmpf.toFilesystemEncoding());
-
        return doVCCommand("cvs -q update "
-               + getTarget(opmode) + redirection,
-               FileName(owner_->filePath()));
+               + getTarget(opmode),
+               FileName(owner_->filePath()), tmpf, false);
 }
 
 
@@ -647,12 +663,14 @@
        int rc = update(File, tmpf);
        string log;
        string const res = scanLogFile(tmpf, log);
-       if (!res.empty())
+       if (!res.empty()) {
                frontend::Alert::error(_("Revision control error."),
                        bformat(_("Error when updating from repository.\n"
                                "You have to manually resolve the conflicts 
NOW!\n'%1$s'.\n\n"
                                "After pressing OK, LyX will try to reopen the 
resolved document."),
                                from_local8bit(res)));
+               rc = 0;
+       }
        
        tmpf.erase();
        return rc ? string() : log.empty() ? "CVS: Proceeded" : "CVS: " + log;
@@ -701,10 +719,23 @@
 
        int rc = update(Directory, tmpf);
        res += "Update log:\n" + tmpf.fileContents("UTF-8");
+       LYXERR(Debug::LYXVC, res);
+
+       string log;
+       string sres = scanLogFile(tmpf, log);
+       if (!sres.empty()) {
+               docstring const file = owner_->fileName().displayName(20);
+               frontend::Alert::error(_("Revision control error."),
+                       bformat(_("Error when updating document %1$s from 
repository.\n"
+                                         "You have to manually resolve the 
conflicts NOW!\n'%2$s'.\n\n"
+                                         "After pressing OK, LyX will try to 
reopen the resolved document."),
+                               file, from_local8bit(sres)));
+               rc = 0;
+       }
+       
        tmpf.removeFile();
 
-       LYXERR(Debug::LYXVC, res);
-       return rc ? string() : "CVS: Proceeded" ;
+       return rc ? string() : log.empty() ? "CVS: Proceeded" : "CVS: " + log;
 }
 
 
@@ -790,9 +821,9 @@
 
 void CVS::getLog(FileName const & tmpf)
 {
-       doVCCommand("cvs log " + getTarget(File)
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+       doVCCommand("cvs log " + getTarget(File),
+                   FileName(owner_->filePath()),
+                       tmpf);
 }
 
 
@@ -888,9 +919,8 @@
        }
 
        LYXERR(Debug::LYXVC, "Detecting locking mode...");
-       if (doVCCommandCall("svn proplist " + quoteName(file_.onlyFileName())
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   file_.onlyPath()))
+       if (doVCCommandCall("svn proplist " + quoteName(file_.onlyFileName()),
+                   file_.onlyPath(), tmpf, FileName()))
                return false;
 
        ifstream ifs(tmpf.toFilesystemEncoding().c_str());
@@ -934,9 +964,8 @@
        }
 
        doVCCommand("svn commit -m \"" + msg + "\" "
-                   + quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+                   + quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
 
        string log;
        string res = scanLogFile(tmpf, log);
@@ -1003,9 +1032,8 @@
                return;
 
        string const arg = lock ? "lock " : "unlock ";
-       doVCCommand("svn "+ arg + quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+       doVCCommand("svn "+ arg + 
quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
 
        // Lock error messages go unfortunately on stderr and are unreachible 
this way.
        ifstream ifs(tmpf.toFilesystemEncoding().c_str());
@@ -1037,9 +1065,8 @@
                return N_("Error: Could not generate logfile.");
        }
 
-       doVCCommand("svn update --non-interactive " + 
quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+       doVCCommand("svn update --non-interactive " + 
quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
 
        string log;
        string const res = scanLogFile(tmpf, log);
@@ -1074,9 +1101,8 @@
                return N_("Error: Could not generate logfile.");
        }
 
-       doVCCommand("svn diff " + quoteName(owner_->filePath())
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-               FileName(owner_->filePath()));
+       doVCCommand("svn diff " + quoteName(owner_->filePath()),
+               FileName(owner_->filePath()), tmpf);
        docstring res = tmpf.fileContents("UTF-8");
        if (!res.empty()) {
                LYXERR(Debug::LYXVC, "Diff detected:\n" << res);
@@ -1105,9 +1131,8 @@
        // + " > " + quoteName(tmpf.toFilesystemEncoding()),
        // FileName(owner_->filePath()));
        // res = "Revert log:\n" + tmpf.fileContents("UTF-8");
-       doVCCommand("svn update --accept mine-full " + 
quoteName(owner_->filePath())
-               + " > " + quoteName(tmpf.toFilesystemEncoding()),
-               FileName(owner_->filePath()));
+       doVCCommand("svn update --accept mine-full " + 
quoteName(owner_->filePath()),
+               FileName(owner_->filePath()), tmpf);
        res += "Update log:\n" + tmpf.fileContents("UTF-8");
 
        LYXERR(Debug::LYXVC, res);
@@ -1130,9 +1155,8 @@
                return N_("Error: Could not generate logfile.");
        }
 
-       int ret = doVCCommand("svn proplist " + 
quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+       int ret = doVCCommand("svn proplist " + 
quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
        if (ret)
                return string();
 
@@ -1141,14 +1165,12 @@
        bool locking = contains(res, "svn:needs-lock");
        if (!locking)
                ret = doVCCommand("svn propset svn:needs-lock ON "
-                   + quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+                   + quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
        else
                ret = doVCCommand("svn propdel svn:needs-lock "
-                   + quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+                   + quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
        if (ret)
                return string();
 
@@ -1246,9 +1268,8 @@
                return N_("Error: Could not generate logfile.");
        }
 
-       doVCCommand("svn info --xml " + 
quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+       doVCCommand("svn info --xml " + 
quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
 
        if (tmpf.empty())
                return false;
@@ -1299,8 +1320,7 @@
                return N_("Error: Could not generate logfile.");
        }
 
-       doVCCommand("svnversion -n . > " + 
quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+       doVCCommand("svnversion -n .", FileName(owner_->filePath()), tmpf);
 
        if (tmpf.empty())
                return false;
@@ -1319,9 +1339,8 @@
 
 void SVN::getLog(FileName const & tmpf)
 {
-       doVCCommand("svn log " + quoteName(onlyFileName(owner_->absFileName()))
-                   + " > " + quoteName(tmpf.toFilesystemEncoding()),
-                   FileName(owner_->filePath()));
+       doVCCommand("svn log " + quoteName(onlyFileName(owner_->absFileName())),
+                   FileName(owner_->filePath()), tmpf);
 }
 
 
@@ -1351,9 +1370,8 @@
        }
 
        doVCCommand("svn cat -r " + revname + " "
-                     + quoteName(onlyFileName(owner_->absFileName()))
-                     + " > " + quoteName(tmpf.toFilesystemEncoding()),
-               FileName(owner_->filePath()));
+                     + quoteName(onlyFileName(owner_->absFileName())),
+               FileName(owner_->filePath()), tmpf);
        if (tmpf.isFileEmpty())
                return false;
 

Reply via email to