Abdelrazak Younes wrote: > Georg Baum wrote: >> Am Samstag, 16. September 2006 18:47 schrieb Abdelrazak Younes: >> >>> I don't need to know. I am just moving code around and this should just >>> work. I am quite confident actually that it _will_ just work. >> >> OK, suppose it works. Then there is still the problem that this is >> supposed to be a cleanup, but obfuscates the code for somebody who knows >> X selection better. > > Such as you?
Partly. I do know a bit about it, but not enough to completely understand the current selection code, in particular not why there is this static xsel_cache_ variable. This is of course not your doing, but a real cleanup would only be possible with understanding the existence of this static variable. > Georg, if you have a comment or an abjection, express it better please. > I didn't committed because I know I can't test and I am counting of > knowledgeable people such as you. As I told, and as you can see from my recent (non-)activity on this list, I don't have the time right now. >> It is standard X terminology to request or clear a >> selection, so a sensible cleanup IMO would be to rename >> selectionRequested to requestSelection and selectionLost to >> clearSelection. > > Well, this new method touch _nothing_ that is related with X11 > (following my cleanup). It just returns the current selection. That is not true (xsel_cache_). > This > method should work for any platform, not only X11. Only X11 has a selection (at least in the sense of selectionRequested). For internal purposes we have e.g. LCursor::selectionAsString. >>> No need for X knowledge, just need someone who test if the selection >>> works or not, that' all. >> >> IMO you need X knowledge if it should be really a cleanup and not >> arbitrary code shuffling around. > > I think that's an unfair comment Georg. I don't think it is unfair. You are changing code related to something you don't use and don't know very well. IMO a real cleanup is only possible with knowledge about the context. In your new name getStringSelection I already see a misunderstanding (I know that you committed the other variant, but I'll show you nevertheless): selectionRequested was only used from the frontend. Your explanation of getStringSelection implies that you think it could also be used for other purposes. That should not be done IMO because of the internal cache. Now you can argue that the current code is not clear either. I completely agree with that, but that does not mean that everything else is better. > If it is just a question of > naming I am all ears. I've named that getStringSelection() because > that's exactly what it does: return the current selection as a string. Nobody says that X terminology is the only useful one, yet it is still known for years. Apart from that the method does not only return a string, but also sets some internal static cache. Georg
