GNOME Bugzilla – Bug 517168
[external-editor] "compose in external editor" not working
Last modified: 2015-06-04 13:36:54 UTC
what should normally happen when i choose "file -> compose in external editor"? nothing happens. shell output is: external_editor plugin is launched ** (evolution:5329): WARNING **: Unable to launch gvim: (well, i understand what it means. but i expect *evolution* [UI] to tell me what's wrong and not silently fail. launch an error message or whatever. and don't expect that more than 5% of the users clicking on this start evo from a shell.) if this is totally experimental, then please remove it from http://live.gnome.org/RoadMap . i guess many distros don't ship experimental. so users will read the roadmap, will get a distro with gnome 2.22, and will wonder where that feature is and file a bug report. hooray. if it's not in experimental but gets shipped by distros not installing gvim by default, evolution fails silently, so users will file a bug report. hooray. i promise i won't triage them anymore.
Yeah, /experimental/ plugins should not be listed in the RoadMap for an upcoming /stable/ GNOME release. The plugin checks the environment variable EDITOR for the name of the editor program to launch, and arbitrarily defaults to "gvim". Several problems here: 1) The choice of editor should be a plugin configuration option in the UI. Defaulting the configuration option to the value of EDITOR is a reasonable thing to do. 2) If EDITOR is not set, fallback to an official GNOME component that's more likely to be installed. GEdit might be a good choice. GVim is not an official GNOME component. There's other issues with that plugin that prevent it from being classified as stable.
(In reply to comment #1) > and arbitrarily defaults to "gvim". Clarification: defaults to "gvim" if EDITOR is not defined.
No, I did the error last time.. Accidently marked backup-restore as experimental. This time External editor shouldn't be experimental. Lack of UI shouldn't make it one. Im asking sankar to get the UI and request for Freeze break.
Another issue with the plugin is the temporary file name is hardcoded as "/tmp/evolution-composer" rather than using g_file_open_tmp() to safely pick a unique name. Apart from the fact that "/tmp" may not be the user's scratch directory (see g_get_temp_dir()), using a hardcoded file name will cause bad things to happen when spawning two or more external editors at once. This is a more serious issue in my mind than any missing UI.
I had no plans of completing this as a standard plugin for 2.22 As a result, I never wrote it as good as I wanted it to be. With the current trunk version, we cannot even spwan two instances of editor. It is a blocking call on main thread. Anyways, I created a new patch that solves all the problems in hopefully the right way.
Created attachment 105752 [details] [review] Fix
I tested the above fix sufficiently and it seems to work fine. I got one crash once when I opened multiple external editors. But I did not get it anytime again. Seems to be fairly stable. Any feedback/comments are welcome. Srag/Matthew: Do you think we need to ask for a freeze break for this patch ?
(In reply to comment #7) > Srag/Matthew: Do you think we need to ask for a freeze break for this patch ? Yeah, I'd say so. One suggestion: In e_plugin_lib_get_configure_widget(), if you get nothing back from the GConf key you could default to the "EDITOR" environment variable as you were doing before. Would be a nice touch to see the plugin default to your favorite editor. If neither are set then leave the label blank. For future versions I wonder if we could offer a choice of applications based on which applications are registered to handle "text/plain" MIME types. In other words make it work like the "Preferred Applications" app. Haven't tested the patch yet... I'll try to later today.
+ <primary>Editor not launcahble</primary> Spell error. Sankar, in any case go for a freeze break and fix this also.
Tested, seems to work well enough for 2.22 once the suggested improvements are implemented. Could use some more UI love before 2.24. I'll defer to Srini for approval.
Created attachment 105893 [details] [review] Updated Patch Waiting for freeze-break request
+ label = gtk_label_new (_("Command to be executed to launch the editor : ")); for one and all times: PLEASE USE PROPER SPELLING. THERE NEVER IS A WHITESPACE IN FRONT OF A COLON. Thanks.
Bumping version to a stable release.
Created attachment 109407 [details] [review] Updated patch incorporating review comments This patch addresses most of the issues that were with the plugin. Now, there might be a crash, because of the new composer not doing well for composer_with_message. I will commit this patch so that it is easy to fix any of the related crasher with the new composer. Committing to TRUNK only.
http://svn.gnome.org/viewvc/evolution?view=revision&revision=35373 I am keeping the bug open until the crasher is resolved.
Setting the right status.
I tried with git master, didn't get any crash. I'm closing this.