Pavel,
I had a look at the locking code (because of bug #6433), but I have some
questions.
Typo ? :
void SVN::scanMaster()
{
locker_.clear();
vcstatus = NOLOCKING;
if (checkLockMode()) {
if (isLocked()) {
locker_ = "Locked";
vcstatus = LOCKED;
} else {
locker_ = "Unlocked";
- vcstatus = LOCKED;
+ vcstatus = UNLOCKED;
}
}
}
> /// The user currently keeping the lock on the VC file.
> std::string locker_;
If the locker_ string contains the user, I would say that the variable
should be named such that it indicates that it is a user name. But then
I see:
> locker_ = "Unlocked";
This is a strange user.
> locker_ = "Locked";
This is also a strange user, but moreover who did lock it then ?
> locker_.clear();
What does it mean when locker_ is empty .. ? then the file is not under
locking policy. But why don't we check for "vcstatus == NOLOCKING" then
? It's a bit strange that we keep this variable and when we want to know
this info, we check for an empty user id.
Furthermore, locker() or locker_ is nowhere used except in the
combination "locker().empty()". Why do we record the user name then ? Or
is this unfinished business ?
> LyXVC.h:
>/// Returns the version number.
>std::string const versionString() const;
>
>VCBackend.h
>
>/// return the current version description
>virtual std::string const versionString() const = 0;
>/// return the current version
>std::string const & version() const { return version_; }
>
>virtual std::string const versionString() const {
> return "RCS: " + version_;
>}
What is a version, version number, version description ? If the only use
of versionString() is to add the "RCS: " part, then I'd rather write
this as VCS::vcName() + " " + VCS::version().
>virtual std::string checkIn(std::string const & msg) = 0;
>virtual std::string checkOut() = 0;
>virtual std::string repoUpdate() = 0;
>virtual std::string lockingToggle() = 0;
I don't understand why these return strings. That's at least very
unintuitive. Why don't we call Buffer::message(string) to show the
status message.
Vincent
