Michael Olson <[EMAIL PROTECTED]> writes: > Stephen Leake <[EMAIL PROTECTED]> writes: > >> Michael Olson <[EMAIL PROTECTED]> writes: >> >>> * lisp/dvc-utils.el (dvc-ewoc-delete): Move the compatibility >>> function for ewoc-delete from dvc-emacs.el to here, and rename it >>> dvc-ewoc-delete. Use it throughout instead of ewoc-delete. This >>> fixes another XEmacs 21.5 issue. >> >> I don't understand this fix. >> >> Previously, we had in dvc-emacs.el: >> >> (if (not (fboundp 'ewoc-delete)) >> (defun ewoc-delete (ewoc &rest nodes) >> ... >> >> Now, we have in dvc-utils.el: >> >> (if (fboundp 'ewoc-delete) >> (defalias 'dvc-ewoc-delete 'ewoc-delete) >> (defun dvc-ewoc-delete (ewoc &rest nodes) >> ... >> >> Why is this better? >> >> It seems to me it is better to use the Emacs name 'ewoc-delete', so >> when XEmacs implements ewoc-delete, we don't have to change so much >> code. > > What I did is better because we should *never* be providing > implementations of functions that could conflict with the namespace of > another package.
ewoc-delete is provided by ewoc.el in Emacs 22.1, but not Emacs 21. Thus it is already in the Emacs namespace; that was the original reason for including it in dvc-emacs, for users of Emacs 21 (including me on some boxes). > If we screw something up, then the developers of another unrelated > package have a very difficult time trying to replicate the bug > reporter's environment, and will be (rightfully) pissed at us. I've > had this happen to me as a developer of Emacs Muse, when another > package provided a bad implementation of replace-regexp-in-string. Was that bad code copied from a release, or from CVS head, or written separately? I think those are different cases, requiring different policies. But I suppose a simple policy is easier to follow and enforce. The code for ewoc-delete was copied verbatim from Gnu Emacs 22. In general, it may be that code for a feature from Emacs 22 is broken in Emacs 21, or in XEmacs. Similarly for code taken from XEmacs. In that case, we need to provide an alternate implementation, or not use the feature, or drop support for Emacs 21. For ewoc-delete, not using the feature is not a choice; it's essential to the notion of cleaning up the diff display without re-running the back-end. In addition, the code is not broken. Although we don't have thorough tests to prove that, it is tested as well as Emacs 22 is in the first place. If code is broken, and two different packages each provide alternate implementations with the same name, each fixing the code to work the way they need it, that would be a problem. I gather that's what happened in the case you cite. > I would like to eventually eliminate dvc-emacs.el and dvc-xemacs.el, > turn them into dvc-compat.el, and change every function definition in > these files to have the prefix "dvc-". Compatibility issues arise in three cases; when there is one name with different functionality in two versions (whether Gnu Emacs 22/21 or Gnu Emacs/XEmacs), when the same feature has different names, and when some feature is simply missing in one version. For the two names case, one policy is to use the Gnu Emacs name, and alias it for XEmacs use. Another is to wrap both with dvc- wrappers, in case it turns out the two functions are not really identical. For the same name/different function case, adding a dvc- wrapper to unify the two is the only reasonable choice. The wrapper may be as simple as an alias on one side. A missing function is essentially the same as the two names case; initially, we provide the code from one version for use in the other, but it may have to be changed if it proves to be broken. Adopting the dvc- wrapper in all cases essentially assumes that all such code is broken; it is defensive programming. I think that is what you are advocating. That ties us permanently to Gnu Emacs 21. Or, if we ever decide to abandon Gnu Emacs 21, we will either be left with a legacy of unnecessary dvc- prefixes, or have to change them to the Emacs 22 names. Given the lack of solid testing in general for Emacs, it does seem that defensive programming is the right choice. There are not so many names in the compatibility packages at the moment, so it won't be a big burden to clean things up in the future. However, I think all such code belongs in a clearly labeled compatibility package, so it can be reviewed periodically for cleanup. That means dvc-ewoc-delete should stay in dvc-emacs.el, not in dvc-utils.el. When we finally drop support for versions of Gnu Emacs and XEmacs that do not provide ewoc-delete, it would be best to delete dvc-ewoc-delete, so the code will then be more understandable. The same goes for other compatibility functions; they should be reviewed at each release of Gnu Emacs and XEmacs to see if they can be deleted. That is much easier to do if they are declared in only one or two files. > I would like to eventually eliminate dvc-emacs.el and dvc-xemacs.el, > turn them into dvc-compat.el, and change every function definition > in these files to have the prefix "dvc-". It does seem like having everything in one file would help. There really isn't that much difference between Gnu Emacs version issues and XEmacs issues. A clear statement of policy in the comments would help. If you agree with what I've said above, I'll add it to both files. Then we can work on eliminating one, and renaming the other. And any other compatibility stuff should be moved to dvc-compat.el, as well. -- -- Stephe _______________________________________________ Dvc-dev mailing list [email protected] https://mail.gna.org/listinfo/dvc-dev
