ubuntu-touch-coreapps-reviewers team mailing list archive

[Arto Jalkanen <ajalkane@xxxxxxxxx>]

Review: Needs Information

See the comment

Diff comments:

> === modified file 'src/plugin/folderlistmodel/smb/qsambaclient/src/smbutil.cpp'
> --- src/plugin/folderlistmodel/smb/qsambaclient/src/smbutil.cpp	2015-05-20 17:15:29 +0000
> +++ src/plugin/folderlistmodel/smb/qsambaclient/src/smbutil.cpp	2015-07-19 15:35:10 +0000
> @@ -545,9 +562,17 @@
>      }//if (fd)
>      else
>      {
> -        qDebug() << Q_FUNC_INFO << "could not open directory" << smb_path << "errno:" << errno;
> +        SHOW_ERRNO(smb_path);
>      }
>      deleteContext(context);
> +    if (!currentPathWithDot.isEmpty())
> +    {
> +        content.append(currentPathWithDot);
> +    }
> +    if (!currentpathWithDotDot.isEmpty())
> +    {
> +        content.append(currentpathWithDotDot);
> +    }

This could be generalized and simplified since the same thing is done in both cases. For example could use just one variable, something like for example "appendToContent" or something

>      return content;
>  }
>  
> @@ -656,7 +681,18 @@
>      Smb::FileHandler fd = openDir(context,smb_path);
>      if (fd == 0)
>      {
> -        fd = openFile(context, smb_path);
> +        openFile(context,smb_path);

Is it a mistake that there's only openFile() without assigning it to filedescriptor?

> +    }
> +    if (fd == 0) // item does not exist neither dir nor file
> +    {
> +        //usually smb_path is a file that does not exist yet
> +        //so using the path
> +        int lastSlash = smb_path.lastIndexOf(QDir::separator());
> +        if (lastSlash != -1)
> +        {
> +             QString path (smb_path.mid(0,lastSlash));
> +             fd = openDir(context,path);
> +        }
>      }
>      if (fd)
>      {


-- 
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-11/+merge/265210
Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app.