The branch, master, has been updated. - Log -----------------------------------------------------------------
commit d33450a5921084677ad50055ad960e199e3a6b3e Author: Georg Baum <[email protected]> Date: Mon Feb 4 21:40:28 2013 +0100 Fix inconsistent VCS checkin return values It is not possible to transport the different error/abort/success conditions of a VCS checkin through a simple empty or nonempty string. Therefore it was no wonder that the return values were used inconsistently for different VCS. Now the log and return status are decoupled, and all VCS are handled consistently. This is a prerequisite for proper locking detection of the upcoming rename command. diff --git a/src/LyXVC.cpp b/src/LyXVC.cpp index d51a905..ff64859 100644 --- a/src/LyXVC.cpp +++ b/src/LyXVC.cpp @@ -173,14 +173,13 @@ bool LyXVC::registrer() } -string LyXVC::checkIn() +LyXVC::CommandResult LyXVC::checkIn(string & log) { LYXERR(Debug::LYXVC, "LyXVC: checkIn"); if (!vcs) - return string(); + return ErrorBefore; docstring empty(_("(no log message)")); docstring response; - string log; bool ok = true; if (vcs->isCheckInWithConfirmation()) ok = Alert::askForText(response, _("LyX VC: Log Message")); @@ -189,15 +188,11 @@ string LyXVC::checkIn() response = empty; //shell collisions response = subst(response, from_ascii("\""), from_ascii("\\\"")); - log = vcs->checkIn(to_utf8(response)); - - // Reserve empty string for cancel button - if (log.empty()) - log = to_utf8(empty); + return vcs->checkIn(to_utf8(response), log); } else { LYXERR(Debug::LYXVC, "LyXVC: user cancelled"); + return Cancelled; } - return log; } @@ -270,9 +265,13 @@ string LyXVC::toggleReadOnly() case VCS::UNLOCKED: LYXERR(Debug::LYXVC, "LyXVC: toggle to locked"); return checkOut(); - case VCS::LOCKED: + case VCS::LOCKED: { LYXERR(Debug::LYXVC, "LyXVC: toggle to unlocked"); - return checkIn(); + string log; + if (checkIn(log) != Success) + return string(); + return log; + } case VCS::NOLOCKING: break; } diff --git a/src/LyXVC.h b/src/LyXVC.h index 5904f19..64ad303 100644 --- a/src/LyXVC.h +++ b/src/LyXVC.h @@ -39,6 +39,13 @@ class Buffer; */ class LyXVC { public: + /// Return status of a command + enum CommandResult { + Cancelled, ///< command was cancelled + ErrorBefore, ///< error before executing command + ErrorCommand, ///< error while executing command + Success ///< command was executed successfully + }; /// LyXVC(); /// @@ -77,8 +84,9 @@ public: // by the next multiple messages on the top of the processed dispatch // machinery. - /// Unlock and commit changes. Returns log. - std::string checkIn(); + /// Unlock and commit changes. + /// \p log is non-empty on success and may be empty on failure. + CommandResult checkIn(std::string & log); /// Does the current VC support this operation? bool checkInEnabled() const; diff --git a/src/VCBackend.cpp b/src/VCBackend.cpp index 3e1bca6..1d18a39 100644 --- a/src/VCBackend.cpp +++ b/src/VCBackend.cpp @@ -233,12 +233,15 @@ void RCS::registrer(string const & msg) } -string RCS::checkIn(string const & msg) +LyXVC::CommandResult RCS::checkIn(string const & msg, string & log) { int ret = doVCCommand("ci -q -u -m\"" + msg + "\" " + quoteName(onlyFileName(owner_->absFileName())), FileName(owner_->filePath())); - return ret ? string() : "RCS: Proceeded"; + if (ret) + return LyXVC::ErrorCommand; + log = "RCS: Proceeded"; + return LyXVC::Success; } @@ -757,22 +760,27 @@ string CVS::scanLogFile(FileName const & f, string & status) ifs.close(); return string(); } - - -string CVS::checkIn(string const & msg) + + +LyXVC::CommandResult CVS::checkIn(string const & msg, string & log) { CvsStatus status = getStatus(); switch (status) { case UpToDate: if (vcstatus != NOLOCKING) - unedit(); - return "CVS: Proceeded"; + if (unedit()) + return LyXVC::ErrorCommand; + log = "CVS: Proceeded"; + return LyXVC::Success; case LocallyModified: case LocallyAdded: { int rc = doVCCommand("cvs -q commit -m \"" + msg + "\" " + getTarget(File), FileName(owner_->filePath())); - return rc ? string() : "CVS: Proceeded"; + if (rc) + return LyXVC::ErrorCommand; + log = "CVS: Proceeded"; + return LyXVC::Success; } case NeedsMerge: case NeedsCheckout: @@ -787,7 +795,7 @@ string CVS::checkIn(string const & msg) toString(status))); break; } - return string(); + return LyXVC::ErrorBefore; } @@ -1158,31 +1166,41 @@ void SVN::registrer(string const & /*msg*/) } -string SVN::checkIn(string const & msg) +LyXVC::CommandResult SVN::checkIn(string const & msg, string & log) { FileName tmpf = FileName::tempName("lyxvcout"); if (tmpf.empty()){ LYXERR(Debug::LYXVC, "Could not generate logfile " << tmpf); - return N_("Error: Could not generate logfile."); + log = N_("Error: Could not generate logfile."); + return LyXVC::ErrorBefore; } - doVCCommand("svn commit -m \"" + msg + "\" " - + quoteName(onlyFileName(owner_->absFileName())) - + " > " + quoteName(tmpf.toFilesystemEncoding()), - FileName(owner_->filePath())); + ostringstream os; + os << "svn commit -m \"" << msg << '"'; + os << ' ' << quoteName(onlyFileName(owner_->absFileName())); + os << " > " << quoteName(tmpf.toFilesystemEncoding()); + LyXVC::CommandResult ret = + doVCCommand(os.str(), FileName(owner_->filePath())) ? + LyXVC::ErrorCommand : LyXVC::Success; - string log; string res = scanLogFile(tmpf, log); - if (!res.empty()) + if (!res.empty()) { frontend::Alert::error(_("Revision control error."), _("Error when committing to repository.\n" "You have to manually resolve the problem.\n" "LyX will reopen the document after you press OK.")); + ret = LyXVC::ErrorCommand; + } else - fileLock(false, tmpf, log); + if (!fileLock(false, tmpf, log)) + ret = LyXVC::ErrorCommand; tmpf.erase(); - return log.empty() ? string() : "SVN: " + log; + if (!log.empty()) + log.insert(0, "SVN: "); + if (ret == LyXVC::Success && log.empty()) + log = "SVN: Proceeded"; + return ret; } @@ -1247,10 +1265,10 @@ string SVN::scanLogFile(FileName const & f, string & status) } -void SVN::fileLock(bool lock, FileName const & tmpf, string &status) +bool SVN::fileLock(bool lock, FileName const & tmpf, string &status) { if (!locked_mode_ || (isLocked() == lock)) - return; + return true; string const arg = lock ? "lock " : "unlock "; doVCCommand("svn "+ arg + quoteName(onlyFileName(owner_->absFileName())) @@ -1266,16 +1284,20 @@ void SVN::fileLock(bool lock, FileName const & tmpf, string &status) } ifs.close(); - if (!isLocked() && lock) + if (isLocked() == lock) + return true; + + if (lock) frontend::Alert::error(_("Revision control error."), _("Error while acquiring write lock.\n" "Another user is most probably editing\n" "the current document now!\n" "Also check the access to the repository.")); - if (isLocked() && !lock) + else frontend::Alert::error(_("Revision control error."), _("Error while releasing write lock.\n" "Check the access to the repository.")); + return false; } diff --git a/src/VCBackend.h b/src/VCBackend.h index 8f2ad41..771d20f 100644 --- a/src/VCBackend.h +++ b/src/VCBackend.h @@ -39,8 +39,10 @@ public: /// register a file for version control virtual void registrer(std::string const & msg) = 0; - /// check in the current revision, returns log - virtual std::string checkIn(std::string const & msg) = 0; + /// check in the current revision. + /// \p log is non-empty on success and may be empty on failure. + virtual LyXVC::CommandResult + checkIn(std::string const & msg, std::string & log) = 0; /// can this operation be processed in the current VCS? virtual bool checkInEnabled() = 0; /// should a log message provided for next checkin? @@ -140,7 +142,8 @@ public: virtual void registrer(std::string const & msg); - virtual std::string checkIn(std::string const & msg); + virtual LyXVC::CommandResult + checkIn(std::string const & msg, std::string & log); virtual bool checkInEnabled(); @@ -214,7 +217,8 @@ public: virtual void registrer(std::string const & msg); - virtual std::string checkIn(std::string const & msg); + virtual LyXVC::CommandResult + checkIn(std::string const & msg, std::string & log); virtual bool checkInEnabled(); @@ -343,7 +347,8 @@ public: virtual void registrer(std::string const & msg); - virtual std::string checkIn(std::string const & msg); + virtual LyXVC::CommandResult + checkIn(std::string const & msg, std::string & log); virtual bool checkInEnabled(); @@ -392,7 +397,7 @@ protected: /// is the loaded file locked? bool isLocked() const; /// acquire/release write lock for the current file - void fileLock(bool lock, support::FileName const & tmpf, std::string & status); + bool fileLock(bool lock, support::FileName const & tmpf, std::string & status); private: /// is the loaded file under locking policy? diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp index 6b9d00d..efd6d43 100644 --- a/src/frontends/qt4/GuiView.cpp +++ b/src/frontends/qt4/GuiView.cpp @@ -2255,7 +2255,7 @@ bool GuiView::renameBuffer(Buffer & b, docstring const & newname) // trying to overwrite ourselves, which is fine.) if (theBufferList().exists(fname) && fname != oldname && theBufferList().getBuffer(fname) != &b) { - docstring const text = + docstring const text = bformat(_("The file\n%1$s\nis already open in your current session.\n" "Please close it before attempting to overwrite it.\n" "Do you want to choose a new filename?"), @@ -2366,7 +2366,8 @@ bool GuiView::exportBufferAs(Buffer & b) } -bool GuiView::saveBuffer(Buffer & b) { +bool GuiView::saveBuffer(Buffer & b) +{ return saveBuffer(b, FileName()); } @@ -2377,7 +2378,7 @@ bool GuiView::saveBuffer(Buffer & b, FileName const & fn) return true; if (fn.empty() && b.isUnnamed()) - return renameBuffer(b, docstring()); + return renameBuffer(b, docstring()); bool success; if (fn.empty()) @@ -2812,8 +2813,14 @@ void GuiView::dispatchVC(FuncRequest const & cmd, DispatchResult & dr) if (!buffer || !ensureBufferClean(buffer)) break; if (buffer->lyxvc().inUse() && !buffer->isReadonly()) { - dr.setMessage(buffer->lyxvc().checkIn()); - if (!dr.message().empty()) + string log; + LyXVC::CommandResult ret = buffer->lyxvc().checkIn(log); + dr.setMessage(log); + // Only skip reloading if the checkin was cancelled or + // an error occured before the real checkin VCS command + // was executed, since the VCS might have changed the + // file even if it could not checkin successfully. + if (ret == LyXVC::ErrorCommand || ret == LyXVC::Success) reloadBuffer(*buffer); } break; ----------------------------------------------------------------------- Summary of changes: src/LyXVC.cpp | 21 ++++++------ src/LyXVC.h | 12 ++++++- src/VCBackend.cpp | 68 +++++++++++++++++++++++++++-------------- src/VCBackend.h | 17 ++++++--- src/frontends/qt4/GuiView.cpp | 17 +++++++--- 5 files changed, 88 insertions(+), 47 deletions(-) hooks/post-receive -- The LyX Source Repository
