Am 02.11.2010 um 12:51 schrieb Stephan Witt:
> Am 02.11.2010 um 12:34 schrieb Vincent van Ravesteijn:
>
>>
>>>>> first step to cure the VCS load problem
>>>> Can I please remind you and ask you to write meaningful commit-logs. Logs
>>>> explaining why the change has been made, why the new design or solution or
>>>> implementation is better, what problem it fixed, why other solutions might
>>>> be wrong although it may seem not to be at first sight and so forth.
>>>>
>>>> In the example of the VCS load problem, please explain what the VCS load
>>>> problem is. Please explain why it is only a first step, and what still
>>>> needs to be done.
>>> The VCS load problem is (was) twofold:
>>> 1. the parser of the document needs the VCS state so this state (checked by
>>> file_found_hook) had to be done earlier.
>>> => first step, move it some lines up, above the readDocument() call.
>>
>> Why does the parser need to know the state ? If the state changes, do we
>> then reparse the file too ?
>
> Yes. The file is reloaded after VCS operations.
>
>> I think this stumbles on a other design problem.
>>
>> If we have a file, we insert an InsetInfo about vcs, then we add the file to
>> a vcs, will the InsetInfo be updated ? Apparently not, apparently the file
>> is updated not until we parse the file again.
>
> See above.
>
>> We probably need to have some call in updateBuffer or something that will
>> update the InsetInfo's, and we call updateBuffer() probably somewhere after
>> loading the file, so we don't need to register the vc status while reading
>> the file.
>
> May be. My plan was to complete CVS backend implementation and the last
> changes did disturb it seriously.
> They where overdue but for my TODO list the worst time to happen. So, I where
> tempted to make the "hot fixes".
>
>>
>>> 2. in case of loading the autosave or emergency file the real file name has
>>> to be passed to file_found_hook.
>>> => the patch I posted does this. (I missed to diff src/buffer_funcs.cpp
>>> and - of course - I'd fix the comments).
>>
>> Well, I made two public functions of Buffer: loadLyXFile and
>> loadThisLyXFile. The first one takes a filename, checks for
>> autosave/emergency files, and when it is sure which file exactly it wants to
>> read it calls loadThisLyXFile(). So, after this point, the code shouldn't be
>> adapted to the fact that LyX is reading a different file as the user
>> requested.
>>
>> The name of the file is already stored in d->filename, so we can just call
>> lyxvc.found_file_hook(d->filename). The only reason for readFile to have a
>> FileName parameter is that we might want to load a recovery/emergency save
>> file instead. This brings me to the conclusion that the FileName parameter
>> to loadLyXFile and loadThisLyXFile is already WRONG!.
>
> I agree.
>
>> We do not set d->filename when calling this file, so we have to make sure
>> that we call loadLyXFile and loadThisLyXFile with exactly the same filename
>> as the one we supplied when creating the buffer. If we rename the buffer we
>> have to explicitly call Buffer::setFileName.
>>
>> To conclude: instead of adding a second parameter to the load*LyXFIle
>> functions, I think we should lose all parameters because we already have
>> this information. The problem with lyxvc only revealed this bug. Thank you
>> for finding the thinko that was present in the code.
>
> Glad to hear this.
>
>> But please don't cure the symptom but criticisize and fix the (re)design.
>
> I did not have the time to follow your redesign of the buffer load operation
> closely.
>
>> Please correct me if I'm wrong. I'm just thinking aloud.
>
> But you cannot test it easily? I'll test your proposal...
The attached patch works.
Stephan
PS. Thank you for the lecture in other mail - (no sarcasm).
My problem is I spend to much time for LyX already. That's why I feel in a
hurry and make mistakes.
Like "solving" a problem by curing the symptom.
Index: src/Buffer.cpp
===================================================================
--- src/Buffer.cpp (Revision 35992)
+++ src/Buffer.cpp (Arbeitskopie)
@@ -876,7 +876,7 @@
}
// InsetInfo needs to know if file is under VCS
- lyxvc().file_found_hook(fn);
+ lyxvc().file_found_hook(d->filename);
if (readDocument(lex)) {
Alert::error(_("Document format failure"),
@@ -3651,7 +3651,6 @@
bool const success = (ret_llf == ReadSuccess);
if (success) {
markDirty();
- lyxvc().file_found_hook(fn);
str = _("Document was successfully recovered.");
} else
str = _("Document was NOT successfully recovered.");
@@ -3707,7 +3706,6 @@
// the file is not saved if we load the autosave file.
if (ret_llf == ReadSuccess) {
markDirty();
- lyxvc().file_found_hook(fn);
return ReadSuccess;
}
return ReadAutosaveFailure;
