@codebrainz commented on this pull request.
> @@ -772,6 +772,36 @@ static gboolean update_config(const
> PropertyDialogElements *e, gboolean new_proj
}
g_free(locale_path);
}
+
+ /* create filename path if it doesn't exist and if file name ends with
'.geany' */
+ if ((err_code = utils_is_file_writable(locale_filename)) != 0 &&
Since the new code doesn't use the value assigned to `err_code` here, doesn't
seem much point in assigning to it.
Moreover, this use of `utils_is_file_writable()` seems like it's deceptively
subtle, given what that function does and the comment above it. It might also
result in a weird race condition.
Since you're using `utils_mkdir(..., TRUE)` below, which will succeed if the
directory already exists, I think you could probably just drop this whole `if`
block. Just get the dirname of the project file and try and create it with
`utils_mkdir(..., TRUE)` as you already do.
Given the above, the logic could probably be restructured like this
(untested/example code):
```c
gchar *project_file_dirname = g_path_get_dirname(locale_filename);
gboolean create_dir = FALSE;
// if the project file's directory doesn't exist, offer the user to create it
if (! g_file_test(project_file_dirname, G_FILE_TEST_EXISTS))
create_dir = dialogs_show_question_full(...);
// if the project file's directory doesn't exist and the user wants to try and
create it
if (create_dir)
{
// try and create the project file's directory recursively, if it doesn't
already exist
err_code = utils_mkdir(project_file_dirname, TRUE);
if (err_code != 0)
{
SHOW_ERR1(...);
g_free(project_file_dirname);
return FALSE;
}
}
else
{
// TODO: should this be handled?
}
g_free(project_file_dirname);
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2586#pullrequestreview-488285588
