Hi Bastiaan, On Thu, Jan 20, Bastiaan Jacques wrote:
The following patch does does two things in selection.c: - - make a_Selection_set_dclick_callback replace the callback instead of returning when the callback was already set. * This function is called only once in the code (in interface.c); the callback pointer always points to the same function. So I don't see a point in merging this first hunk, although it won't do any harm. Thoughts?
I'd rather leave it the current way, since changing the callback as in your patch would mean, that the old callback would be silently deactivated. To extend the current functionality, a remove function may be useful (to make deactivation explicit), or even a list of callbacks, so that the functions do not have to be removed (unless they should so). Anyway, there is currently no demand for an extended functionality, so I'd leave it as simple as it is now.
- - Replace three comparison operators (==) with bitwise AND.. * This only works as expected when either lvalue and rvalue have exactly one or zero bits set to 1. Although this currently is true, this may change in the future; thus, I don't think these three hunks should be merged.
This (that only one bit set) will also be true in the future. If contents will be extended to contain multiple types (which is not planned at all), the whole design must be changed anyway (at least, the unions will have to replaced by structs), so grepping over the code is anyway necessary. Until then, your suggestion would only make the code more confusing. Sebastian