Re: r35575 - lyx-devel/trunk/src

2010-10-15 Thread Stephan Witt
Am 15.10.2010 um 18:00 schrieb Pavel Sanda:

> Pavel Sanda wrote:
>> this is already bug #chrrm (bugzilla is down). 
> 
> #6396 

Ok, thanks.

Stephan


Re: r35575 - lyx-devel/trunk/src

2010-10-15 Thread Pavel Sanda
Pavel Sanda wrote:
> this is already bug #chrrm (bugzilla is down). 

#6396 


Re: r35575 - lyx-devel/trunk/src

2010-10-15 Thread Pavel Sanda
Stephan Witt wrote:
> But in general owner_->fileName() should be used instead of file_.

yes

> Another question: when the working copy has no diff to repository
> the check in operation isn't sensible. But it's expensive to test
> this. So I'd like to test it before prompting for the log message
> and then doing the noop check in. Does this sound sensible?

this is already bug #chrrm (bugzilla is down). i was about to fix it few months
back but there was some catch (hard to remember, maybe because checkin can
release locks only or something like that), so i postponed it ;)

i agree this should be fixed but dont have time recheck that all other rcs/svn
usecases are not affected, so this testing would be up to you.

pavel


Re: r35575 - lyx-devel/trunk/src

2010-10-15 Thread Stephan Witt
Am 15.10.2010 um 15:08 schrieb Pavel Sanda:

> Pavel Sanda wrote:
>> i quickly went through sources and saw also few usage of file_ outside
>> scanMaster in svn routines which looks inconsitent and owner_ would be
>> more appropriate.
> 
> no as i see it again they are part of scanMaster so the question really is 
> whether
> this routine can be called without existing buffer...

AFAICS not. scanMaster() gets called from constructors only.
The constructors are in LyXVC::registrer() which gets called from buffer
and LyXVC::file_found_hook() - called when buffer file is readable.

So I think file_ is used as internal state to transfer the constructor
parameter to scanMaster. Since it's not really broken - I'll refrain
from fixing it.

But in general owner_->fileName() should be used instead of file_.

Another question: when the working copy has no diff to repository
the check in operation isn't sensible. But it's expensive to test
this. So I'd like to test it before prompting for the log message
and then doing the noop check in. Does this sound sensible?
The proper solution would be to refactor the diff operation and
use it for that and for repoUpdate in the check for it and prompt
when a real change was made.
If you're fine with that I'll prepare a patch otherwise I have to
copy the diff test code to CVS::checkIn.

Stephan

Re: r35575 - lyx-devel/trunk/src

2010-10-15 Thread Pavel Sanda
Pavel Sanda wrote:
> i quickly went through sources and saw also few usage of file_ outside
> scanMaster in svn routines which looks inconsitent and owner_ would be
> more appropriate.

no as i see it again they are part of scanMaster so the question really is 
whether
this routine can be called without existing buffer...

pavel


Re: r35575 - lyx-devel/trunk/src

2010-10-15 Thread Pavel Sanda
Stephan Witt wrote:
> The owner_ variable "points" to the buffer, of course. 
> Most of the time VC implementation uses the filename of the buffer.
> But I cannot see what sense the file_ variable of the VC instance has.

intuitively the meaning is clear - when possible use owner. when buffer
might/does not exist use filename. there are two cases i can imagine
where buffer does not exist - calling scanMaster before loading the
file and when we reload the document. reload has been refactorized
during 2.0svn development and iirc buffer structure is not destructed now.
whether scanMaster is actually called before buffer creation i would
need to check. otherwise i wouldnt see need for file_.

> One can drop it - or use it consequently - I'd say.
>  
> > i would need to dig into the code if you want some
> > specific info.
> 
> If possible... that would be nice.

i quickly went through sources and saw also few usage of file_ outside
scanMaster in svn routines which looks inconsitent and owner_ would be
more appropriate.

pavel


Re: r35620

2010-10-15 Thread Jürgen Spitzmüller
Uwe Stöhr wrote:
> Jürgen,
> 
> can this also go to branch?

Yes.

Jürgen


Re: Another patch for 1.6.x

2010-10-15 Thread Jürgen Spitzmüller
Stephan Witt wrote:
> I have another patch in my local branch checkout...
> It fixes a warning and a potential bug.

OK.

Jürgen