Re: [PATCH] svn_fs_text_changed

Chia-liang Kao <clkao@clkao.org> writes:

> > Hm. Nasty. Why did you add a new public FS interface which does
> > nothing but call other public FS interfaces and is used in exactly one
> > place in the code? Bad plan, homey. Your svn_fs_text_changed()
> > implementation should just be a helper function in delta.c.
>
> I don't have any strong opinion whether this should be a public api,

That works out great -- because I do. :-)

> i just think that could be useful somehow. perhaps it could be a
> helper function in svn_io.h that wc's versioned_file_modified_p
> could also use.

Er... no. Just make it a helper in delta.c. If later we want to
expose it publicly, I suspect that svn_repos_compare_files() would be
a nice name and location for it.

> Adding md5 checks before the byte-by-byte checks could catch the
> differences earlier between some fixed-size binaries.

Yep. That'd be a good change.

> Well, it seems to me more like a consistencies fix for editors: today
> they already know if open_file is called but no apply_textdelta called,
> it means the file content isn't modified.

Ah, but this is slightly different. You're losing information here,
namely the difference between "this file's contents haven't been
changed" and "this file's contents have been changed, but happen to
look just like those of another file". Is this difference
significant? I really dunno.

You are effectively changing this code only for 'diff' and 'merge',
though, so perhaps its a safe change to make.

I'm still curious to see some real timings. I'd love to know what
would happen if you made this change for all uses of dir_delta().

> While with a wrapper to window_handler, you'll have to change every
> editor, and have call stack overhead for every window, although also
> insignificant.

Wrapper? The window_handler() just sez "If this is the first
window, and it's NULL, there's nothing to do." This is a baton
examination, nothing more.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Nov 25 16:35:59 2003