Code review comment for lp:~nathanael-naeri/deja-dup/fix-1549776

Revision history for this message
Naël (nathanael-naeri) wrote :

Thanks for the review and the merge Michael. I think dropping the else condition is a good idea indeed. However, it results in a metric ton of errors when I run "make check". I think it's just because what was in the else:

  result = dir.replace("$USER", Environment.get_user_name());

now overwrites "result" unconditionally with "dir.replace", so the benefit of all the previous if-conditions is lost. But we can't simply use:

  result = result.replace("$USER", Environment.get_user_name());

either because then "result" is "null" if none of the previous if-conditions is triggered.

Below is a possible fix to this issue (can be applied to current revision 1572 but beware of Launchpad line wrapping), but there are many others I think, for instance initializing "result" with "dir" instead of "null" and then using "result = result.replace" should work too.

----- BEGIN PATCH -----
=== modified file 'libdeja/DirHandling.vala'
--- libdeja/DirHandling.vala 2017-02-13 22:18:16 +0000
+++ libdeja/DirHandling.vala 2017-02-19 21:05:35 +0000
@@ -52,9 +52,11 @@
     result = dir.replace("$TRASH", get_trash_path());
   else if (dir.has_prefix("$VIDEOS"))
     result = dir.replace("$VIDEOS", Environment.get_user_special_dir(UserDirectory.VIDEOS));
+ else
+ result = dir;

   // Some variables can be placed anywhere in the path
- result = dir.replace("$USER", Environment.get_user_name());
+ result = result.replace("$USER", Environment.get_user_name());

   // Relative paths are relative to the user's home directory
   if (Uri.parse_scheme(result) == null && !Path.is_absolute(result))
----- END PATCH -----

« Back to merge proposal