GNOME Bugzilla – Bug 125342
Cut/Copy/Paste should be insensitive when not available
Last modified: 2009-08-15 18:40:50 UTC
You can use cong_range_can_be_cut to determine if the document's selection's ordered range can be cut or copied. You can connect to the "selection_change" signal of the CongDocument. This should work for the menus and the toolbar, though possibly not for the popup menus. See cong-primary-window.c, and cong-menus.c
Patch for Edit menu sent to mailing list. Any ideas what the Paste button should be attached to? Where is paste-able info stored, can we check to see if that's empty and set sensitivity based on that?
Have a look in src/cong-app.c: it gets the clipboard content from X (so that you can paste content from other applications). This means that you'd have to attach a notification to the X clipboard content being changed. I _think_ there's a way of doing this in GTK. You should implement a "can_paste" boolean function since there are other conditions about whether a paste can occur. The cursor needs to be at a sane location, and the clipboard needs to have content, which needs to be able to be parsed into XML nodes. Do a grep for cong_document_paste_clipboard_or_selection, and cong_document_paste_source_at, this should provide the information you need, together with an idea of how to implement this easily.
Have edited my working copy to de-sensitize the toolbar Copy and Cut under the same conditions as the Edit menu items, but as it requires a few changes to cong-menus.c I have to wait for the anonymous CVS server to catch up so I can make a diff! Will attach it tomorrow. Meant to comment earlier - As far as cong_range_can_be_cut goes, is there any difference between cong_selection_get_logical_range and it's ordered equivalent? I used logical ranges throughout these patches but also put FIXME comments where cong_selection_get_logical_range is called. If this is indeed correct, just remove the FIXME comments (or ask me to do it :)
It's an invariant that logical_range and ordered_range contain the same two locations; the ordered_range contains them in "document order" and so they may be reversed w.r.t. the logical_range. The logical range is only important when doing things like dragging out the selection, when it's important to know which end of the selection was the start, and which is the end being dragged, so that you can drag "backwards"). Most things use the ordered range, as it simplifies the code for handling things like "cut". So I don't think it matters which you use as regards the predicates for sensitivity. It's a pain about anon CVS; what happens if you try and make a patch regardless?
Created attachment 21108 [details] [review] Desensitizes Cut/Copy toolbar icons when not available
Well, I made the above patch using anon CVS anyway, there's three files in there, I believe you'll find ChangeLog and cong-primary-window.c patches should apply no problem, but the diff for cong-menu.c is against revision 1.41, which is immediately before the patch I sent to the mailing list. Importantly, the path to cong-menus.c above and the patch I sent to the mailing list are ever so slightly different, I only changed the 2 references to on_selection_changed_copy to on_selection_changed_copy_menu (and the same for cut) so that I fit in more transparently with the current naming scheme for undo and redo. If you can figure out how to apply the patch to cong-menus.c then great, if not I'll resubmit when anon CVS catches up. (I suppose you could back out my earlier patch on cong-menus.c then apply this one, that should work) Now to get to work on the Paste button :)
Thanks for the comment regarding logical/ordered ranges, it turns out that cong_range_can_be_cut only works as expected on an ordered range. I'll submit (another!) patch when anon CVS catches up.
Created attachment 21110 [details] [review] Desensitizes Cut/Copy toolbar icons when not available
OK, anon CVS caught up, don't use the patch at the top (21108) use this one instead (21110) This patch is against an up to date CVS, and also works properly if the selection is dragged backwards. Can't close the bug yet, Paste is still not done.
OK - I've applied 21110 to CVS. Looking forward to a "paste" fix!
Created attachment 21125 [details] [review] Desensitizes cut/copy/paste in context menu, and paste in Edit menu and Toolbar when unavailable
Lots going on in above patch! Firstly, the same rules for desensitizing the Cut/Copy items have been applied to the RMB context-menu. That should wipe out the last case of cut or copy being called without a vaild range, so asserts have been added to xmledit.c in place of the "FIXME: UI should be insensitive" comments. Next, the cong_range_can_be_copied function has been implemented, and some places that use can_be_cut where they should use can_be_copied have been updated. At the moment, can_be_copied simply calls can_be_cut, so results are the same. I believe this should change in the future. Finally, Paste has been desensitized in all 3 locations (Edit menu, Toolbar and Context menu) according to the result of cong_document_can_paste(CongDocument). The body of this function has mainly come from cong_document_paste_clipboard_or_selection in xmledit.c but will probably need more work. A couple of comments: 1) Should the can_paste function be in cong-document.c? (I couldn't think of a more logical place to put it, I don't think it should be in cong-range with can_be_cut and can_be_copied. 2) The paste items have been connected to the document selection_changed event, but should also be connected to an event when the clipboard contents change (still to do) 3) When I try to paste anything, even simple text from within the same document, I get a crash with: ** ERROR **: file cong-node.c: line 96 (cong_node_type): should not be reached aborting... I don't think this has anything to do with the above patch, should this go in as a separate bug report? Any feedback welcome!
Created attachment 21126 [details] [review] Changelog for above patch
Regarding Comment 3) above Checked out a clean CVS, Paste works fine. Crash *is* caused by the above patch. Finding out why now.
Created attachment 21128 [details] Conglomerate ouput before patch
Created attachment 21129 [details] Conglomerate ouput after patch
This has me foxed! Here's what I did: 1) Check out a clean copy of Conglomerate from CVS. 2) Open Conglomerate with README.xml file 3) Select word Conglomerate in first paragraph 4) Hit Copy toolbar icon 5) Move cursor past word "structural" in same paragraph 6) Hit Paste toolbar icon 7) Close Conglomerate 8) Apply above patch 9) Repeat steps 2-6 above Step 6 then causes a crash. I recorded the output from conglomerate before and after applying the patch, and I'm getting several failed assertions, but I can't see why my patch causes them. (See above attachments) After the patch, I'm getting several assertions of the form: ** (conglomerate:28712): CRITICAL **: file cong-document.c: line 813 (cong_document_begin_command): assertion `NULL==PRIVATE(doc)->current_command' failed ** (conglomerate:28712): CRITICAL **: file cong-command.c: line 400 (cong_command_add_cursor_change): assertion `IS_CONG_COMMAND(cmd)' failed ** (conglomerate:28712): CRITICAL **: file cong-command.c: line 382 (cong_command_add_selection_change): assertion `IS_CONG_COMMAND(cmd)' failed ** (conglomerate:28712): CRITICAL **: file cong-document.c: line 859 (cong_document_end_command): assertion `IS_CONG_COMMAND(cmd)' failed These assertions occure whilst moving the cursor or dragging out the selection. The Cut command appears to work successfully, but the Paste fails with: ** Message: (454,310) -> index 75 ** Message: cong_command_history_add_command ("Move cursor") ** Message: <?xml version="1.0"?> <placeholder>Conglomerate</placeholder> ** ERROR **: file cong-node.c: line 96 (cong_node_type): should not be reached aborting... Any offerings on this would be greatly appreciated. I'm going to split the above rather large patch into several smaller ones and try to figure out the line that causes the crash.
Created attachment 21132 [details] [review] Desensitizes cut/copy/paste in context menu, and paste in Edit menu and Toolbar when unavailable (without crashing)
Firstly, my apologies for the sheer volume of noise coming from this bug. I dissected the patch line by line, and it was the code that checked the clipboard which caused the assertions, so it has been removed. The revised patch has the same function: Desensitizes Cut/Copy in the RMB context menu Implements cong_range_can_be_copied Replaces erroneous calls to cong_range_can_be_cut with cong_range_can_be_copied Desensitizes Paste in all 3 locations (edit menu, context menu, toolbar) when cursor is in an invalid location. DOES NOT Desensitize Paste when Clipboard is empty. In fact, doesn't check clipboard at all. It should not now be possible to call Cut/Copy on a non-cut/copyable range, so the checks for valid ranges have been replaced with asserts. My suggestions: 1) ignore patch in attachment 21125 [details] [review]/21126 2) apply patch in attachment 21132 [details] [review] 3) close this bug 4) open new bug along lines of "Paste should be desensitized when clipboard empty or invalid" referring to cong_document_can_paste in cong-document.c I suggest the closing/reopening to make the bug more visible, I've made this thread too noisy.
"reopening" doesn't make a bugreport more clear. Create a new one and close this one as a duplicate of the new. (every registered bugzilla user can open a bugreport.
Although I can make a new report, I can't close this one off. Also, a new bug report isn't really justified unless the patch in attachment 21132 [details] [review] is applied, which I also can't do. Again, sorry to have made this report so messy in the first place.
"you can't make an omelet without breaking eggs" So, don't worry about the mess :-) Happy hacking --Geert Stappers
Neither of the above patches are any good - the second one seems to have been made against some older files in CVS (Bet that's the Anon server not catching up again) Give the server a few days, and I'll re-submit patches that stand a chance of being applied.
Bugs #126087, #126089, and #126091 have been created as separate paths down which this bug leads. First one is about Cut/Copy being insensitive in the context menu Second one is about Paste being insensitive in all 3 place (Edit, toolbar, context) Third is about Cut and Copy being available under different conditions. All three new bugs have reference back to here. I have patches for all three, made by breaking up the above monolithic patches, and will submit them in due course (i.e. when Anon CVS catches up properly). I don't have privileges to close down this bug, and I wouldn't know the best way to close it down anyway. The choice is yours :)
Cloned into bug 126087, bug 126089, and bug 126091
All the clones are crossreferenced. Closing it.
The line here: ** (conglomerate:28712): CRITICAL **: file cong-document.c: line 813 (cong_document_begin_command): assertion `NULL==PRIVATE(doc)->current_command' failed means that you have mismatched calls to cong_document_begin/end_command (i.e. you're starting a new command before finishing the old one)