ubuntu-touch-coreapps-reviewers team mailing list archive

[Arto Jalkanen <ajalkane@xxxxxxxxx>]

Review: Needs Information



Diff comments:

> === modified file 'src/plugin/folderlistmodel/dirmodel.cpp'
> --- src/plugin/folderlistmodel/dirmodel.cpp	2015-03-08 12:04:13 +0000
> +++ src/plugin/folderlistmodel/dirmodel.cpp	2015-03-08 12:04:13 +0000
> @@ -881,13 +881,9 @@
>  bool  DirModel::cdIntoIndex(int row)
>  {
>      bool ret = false;
> -    if (IS_VALID_ROW(row)                       &&
> -        mDirectoryContents.at(row).isDir()      &&
> -        mDirectoryContents.at(row).isContentReadable())
> +    if (IS_VALID_ROW(row))
>      {
> -        mCurLocation->setInfoItem(mDirectoryContents.at(row));
> -        setPathFromCurrentLocation();
> -        ret = true;
> +        ret = cdIntoItem(mDirectoryContents.at(row));
>      }
>      else
>      {
> @@ -896,42 +892,29 @@
>      return ret;
>  }
>  
> -
> +/*!
> + * \brief DirModel::cdIntoPath()  It is used to go into an item from the current path or to a absolute path
> + * \param filename
> + * \return
> + */
>  bool  DirModel::cdIntoPath(const QString &filename)
>  {
> -    bool ret = false;
> -    DirItemInfo fi(filename);
> -    if (fi.isValid())
> -    {
> -        if (fi.isRelative())
> -        {
> -            fi.setFile(mCurrentDir, filename);
> -        }
> -        ret =  cdInto(fi);
> -    }
> -    return ret;
> +    return openPath(filename);
>  }
>  
>  
> -bool  DirModel::cdInto(const DirItemInfo &fi)
> +bool  DirModel::cdIntoItem(const DirItemInfo &fi)
>  {
>      bool ret = false;
> -    if (canReadDir(fi.diskFileInfo()))
> -    {
> -        if (fi.isRelative())
> -        {
> -            QDir childDir(mCurrentDir);
> -            if ( childDir.cd(fi.fileName()) )
> -            {
> -                setPath(childDir.absolutePath());
> -                ret = true;
> -            }
> -        }
> -        else
> -        {
> +    if (fi.isBrowsable())
> +    {       
> +        if (fi.isContentReadable())
> +        {
> +            mCurLocation->setInfoItem(fi);
> +            setPathFromCurrentLocation();
>              ret = true;
> -            setPath(fi.absoluteFilePath());
>          }
> +
>      }
>      return ret;
>  }
> @@ -1282,7 +1265,7 @@
>      bool ret = false;
>      if (IS_VALID_ROW(row))
>      {
> -        if (mDirectoryContents.at(row).isDir())
> +        if (mDirectoryContents.at(row).isBrowsable())
>          {
>              ret = cdIntoIndex(row);
>          }
> @@ -1298,27 +1281,35 @@
>      return ret;
>  }
>  
> +
>  bool DirModel::openPath(const QString &filename)
>  {
>      bool ret = false;
> -    //first void any relative path when is root
> -    if ( !(mCurLocation && mCurLocation->isRoot() && filename.startsWith(QLatin1String(".."))) )
> +    QString myFilename(filename.trimmed());
> +    //first avoid any relative path when is root
> +    if ( !(mCurLocation && mCurLocation->isRoot() && myFilename.startsWith(QLatin1String(".."))) )
>      {
> -        Location *location = mLocationFactory->setNewPath(filename);
> -        if (location)
> +        if (myFilename == QLatin1String("..") || myFilename == QLatin1String("../"))
>          {
> -            mCurLocation = location;
> -            setPathFromCurrentLocation();
> -            ret = true;
> +            ret = cdUp();
>          }
>          else
>          {
> -           const DirItemInfo *item = mLocationFactory->lastValidFileInfo();
> -           // DirItemInfo fi(setParentIfRelative(filename));
> -           if (item && item->isFile())
> -           {
> -               ret =  openItem(*item);
> -           }
> +            Location *location = mLocationFactory->setNewPath(myFilename);
> +            if (location)
> +            {
> +                mCurLocation = location;
> +                setPathFromCurrentLocation();
> +                ret = true;
> +            }
> +            else
> +            {
> +                const DirItemInfo *item = mLocationFactory->lastValidFileInfo();               
> +                if (item && item->isFile())
> +                {
> +                    ret =  openItem(*item);

I'm not sure if I understand what happens here correctly, but if I do, then this can't be right: does this mean that if we have just clicked a file to open it, and then try to change to some other directory that is not valid... then the file that was previously opened is opened again?

> +                }
> +            }
>          }
>      }
>      return ret;
> @@ -1332,19 +1323,16 @@
>  bool DirModel::openItem(const DirItemInfo &fi)
>  {
>      bool ret = false;
> -    if (fi.isLocal())
> -    {
> -        if (canReadDir(fi.diskFileInfo()))
> -        {
> -            ret = cdInto(fi.diskFileInfo());
> -        }
> -        else
> -        {
> -            //TODO open executables
> -            if (canReadFile(fi.diskFileInfo()))
> -            {
> -                ret = QDesktopServices::openUrl(QUrl::fromLocalFile(fi.absoluteFilePath()));
> -            }
> +    if (fi.isBrowsable())
> +    {
> +        ret = cdIntoItem(fi);
> +    }
> +    else
> +    {
> +        //TODO open executables
> +        if (fi.isLocal() && fi.isReadable())
> +        {
> +            ret = QDesktopServices::openUrl(QUrl::fromLocalFile(fi.absoluteFilePath()));
>          }
>      }
>      return ret;
> 
> === modified file 'src/plugin/folderlistmodel/dirmodel.h'
> --- src/plugin/folderlistmodel/dirmodel.h	2015-03-08 12:04:13 +0000
> +++ src/plugin/folderlistmodel/dirmodel.h	2015-03-08 12:04:13 +0000
> @@ -459,7 +459,7 @@
>      int           rowOfItem(const DirItemInfo& fi);
>      QDir::Filter  currentDirFilter()  const;
>      QString       dirItems(const DirItemInfo& fi) const;
> -    bool          cdInto(const DirItemInfo& fi);
> +    bool          cdIntoItem(const DirItemInfo& fi);
>      bool          openItem(const DirItemInfo& fi);  
>      bool          canReadDir(const QFileInfo& d)   const;
>      bool          canReadFile(const QFileInfo& f)  const;
> 


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