Summary: | code import wizard: does not read folders | ||
---|---|---|---|
Product: | [Applications] umbrello | Reporter: | arne anka <kde-bugs> |
Component: | general | Assignee: | Joris Steyn <kde> |
Status: | RESOLVED INTENTIONAL | ||
Severity: | normal | CC: | kde, ralf.habacker |
Priority: | NOR | ||
Version: | 2.11.3 | ||
Target Milestone: | --- | ||
Platform: | Debian unstable | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/umbrello/7be6bf1bf86bea9ecc6e4ff4cfbe114c59e6aced | Version Fixed In: | |
Sentry Crash Report: |
Description
arne anka
2013-11-16 20:35:35 UTC
A debugging session shows that QFileSystemModel::row() returns zero if the related dir is not expanded, see below: void CodeImpSelectPage::treeClicked(const QModelIndex& index) { .... uDebug() << "item at row=" << index.row() << " / column=" << index.column(); QFileSystemModel* indexModel = (QFileSystemModel*)index.model(); QFileInfo fileInfo = indexModel->fileInfo(index); if (fileInfo.isDir()) { // QFileSystemModel returns zero childrens for a collapsed directory int rows = indexModel->rowCount(index); ... The code import wizard has code to select and count file recursivly with selected "include subdirs", but it fails because of the zero childs mentioned above. QFileInfo childInfo = indexModel->fileInfo(childIndex); if (childInfo.isDir() && ui_subdirCheckBox->isChecked()) { treeClicked(childIndex); } I found a partial workaround by expanding the directory before fetching the children. if (fileInfo.isDir()) { // QFileSystemModel returns zero childrens for a collapsed directory ui_treeView->setExpanded(index, true); qApp->processEvents(); int rows = indexModel->rowCount(index); unfortunally a few tests show that this fix is not enough to set this bug as resolved. Git commit c2bba32636e3b7b8646dd01008f70efab8c86d02 by Joris Steyn. Committed on 25/02/2014 at 18:06. Pushed by jorissteyn into branch 'master'. Sort files ascending by name in code import wizard M +4 -0 umbrello/codeimpwizard/codeimpselectpage.cpp http://commits.kde.org/umbrello/c2bba32636e3b7b8646dd01008f70efab8c86d02 QFileSystemModel does lazy loading of directory contents, so the checkbox only works if the parent directory has been previously expanded by the user - which is very confusing behaviour. The wizard does no real recursive scanning of directories. Loading and selecting all child items when clicking on a directory item is not easy to implement: fetchMore() works asynchronously. So I tried to split up the recursive/non-recursive selection in a way that allows selecting individual files (current behaviour) as well as (what most users expect / need) just a single directory. Ralph, I'll push this to work/327701. Could you check if this works alright for you? Maybe you've got a better idea? Git commit 7be6bf1bf86bea9ecc6e4ff4cfbe114c59e6aced by Joris Steyn. Committed on 27/02/2014 at 20:19. Pushed by jorissteyn into branch 'work/327701'. Import wizard allows recursive import of directory Selecting sparse files (old behaviour) is still possible by unchecking the checkbox. The recursive behaviour seems like the most common use case so the option is checked by default. Changes in this commit: * rename the 'include subdirectories' option to 'recursive directory import' * when checked, the tree view shows only directories * when checked, only a single item can be selected. The file counter counts all files contained in the selected directory (recursively). * the 'select all' button has been removed (didn't work, don't see a use case for it) M +62 -106 umbrello/codeimpwizard/codeimpselectpage.cpp M +5 -3 umbrello/codeimpwizard/codeimpselectpage.h M +7 -11 umbrello/codeimpwizard/codeimpselectpage.ui http://commits.kde.org/umbrello/7be6bf1bf86bea9ecc6e4ff4cfbe114c59e6aced (In reply to comment #3) > QFileSystemModel does lazy loading of directory contents, so the checkbox > only works if the parent directory has been previously expanded by the user - > which is very confusing behaviour. agree >The wizard does no real recursive scanning of directories. > Loading and selecting all child items when clicking on a directory item is > not easy to implement: fetchMore() works asynchronously. So I tried to split up > the recursive/non-recursive selection in a way that allows selecting individual > files (current behaviour) as well as (what most users expect / need) just a single > directory. > > Ralph, I'll push this to work/327701. Could you check if this works alright > for you? works but has issues see below. Maybe you've got a better idea? We should take a look at the related use cases. 1. With bug 315417 for example the reporter requested to import a single or less number of files, which is suboptimal with the recent import wizard. Much faster is to use the logical View context menu entry "import file(s)", which has been implemented on the request and because I personally also missed this feature. 2. Importing a whole project (which is defined as "all subdirectories of a given dir") is also much faster by using the logical View context menu entry "import project". 3. What is left over is a use case for selecting a mix of directories and files, for which the recent dialog is also suboptimal because of the little size and overall handling. The system provided file or directory selection dialogs are much better in this and other areas like recent path list, different views, net access, size handling and common ui style and I don't think that we like or should reimplement this in umbrello. I think it is always better to use system provided dialogs because of the native look and feel and to extend them if possible and required. I personal like the selected files number counting you have fixed, but how long does it take if someone select the root dir of a TB big network share ? I did this with my local 90GB big linux root dir; it let the wizard freeze for several minutes and I have to kill umbrello. :-( Would it be possible to avoid any freezes especial when using network shares ? If no we should drop this completly. stopped using it a long time ago |