OK, before I apply this patch, about some stylistic nits? :-)
{
- const char *editor, *cmd;
+ const char *editor, *cmd, *base_dir, *file_name, *base_dir_apr;
+ char *old_cwd;
int sys_err;
+ apr_status_t apr_err;
+ svn_error_t *err = SVN_NO_ERROR;
Our convention, usually, is to end all functions with 'return SVN_NO_ERROR'.
+ base_dir = svn_path_dirname(path, pool);
+ file_name = svn_path_basename(path, pool);
Couldn't you do this all at once with a single call to svn_path_split()?
+
SVN_ERR(find_editor_binary(&editor, editor_cmd, config));
- cmd = apr_psprintf(pool, "%s %s", editor, path);
+ apr_err = apr_filepath_get(&old_cwd, APR_FILEPATH_NATIVE, pool);
+ if (apr_err)
+ return svn_error_wrap_apr(apr_err, _("Can't get working directory"));
+
+ /* APR doesn't like "" directories */
+ if (base_dir[0] == '\0')
+ base_dir_apr = ".";
+ else
+ SVN_ERR(svn_path_cstring_from_utf8(&base_dir_apr, base_dir, pool));
+
+ apr_err = apr_filepath_set(base_dir_apr, pool);
+ if (apr_err)
+ {
+ return svn_error_wrap_apr
+ (apr_err, _("Can't change working directory to '%s'"), base_dir);
+ }
No need for curly braces around a single-line block.
+
+ cmd = apr_psprintf(pool, "%s \'%s\'", editor, file_name);
sys_err = system(cmd);
if (sys_err != 0)
- /* Extracting any meaning from sys_err is platform specific, so just
- use the raw value. */
- return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
- _("system('%s') returned %d"), cmd, sys_err);
+ {
+ /* Extracting any meaning from sys_err is platform specific, so just
+ use the raw value. */
+ err = svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
+ _("system('%s') returned %d"), cmd, sys_err);
+ }
Same thing here.
- return SVN_NO_ERROR;
+ apr_err = apr_filepath_set(old_cwd, pool);
+ if (apr_err)
+ {
+ svn_handle_error2(svn_error_wrap_apr
+ (apr_err, _("Can't restore working directory")),
+ stderr, TRUE /* fatal */, "svn: ");
+ }
+
+ return err;
}
Same thing here.
Actually, why do you define 'err' at the top of the function at all?
I don't see it used.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 22 19:54:08 2007