Bug 113759 - [PATCH] Folder quick filing feature for KMail
Summary: [PATCH] Folder quick filing feature for KMail
Status: RESOLVED FIXED
Alias: None
Product: kmail
Classification: Unmaintained
Component: general (show other bugs)
Version: unspecified
Platform: RedHat Enterprise Linux Linux
: NOR wishlist
Target Milestone: ---
Assignee: Till Adam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-03 06:20 UTC by Kumaran Santhanam
Modified: 2009-12-06 17:00 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Enhanced folder selection dialog (25.03 KB, image/png)
2005-10-03 06:21 UTC, Kumaran Santhanam
Details
Patch to implement enhanced folder selection feature (9.96 KB, patch)
2005-10-03 06:23 UTC, Kumaran Santhanam
Details
Patch for version 3.5.0 (10.36 KB, patch)
2006-01-20 06:31 UTC, Kumaran Santhanam
Details
Patch for version 3.5.2 (11.79 KB, patch)
2006-05-22 01:46 UTC, Kumaran Santhanam
Details
Patch for version svn-20060521 (9.85 KB, patch)
2006-05-22 01:49 UTC, Kumaran Santhanam
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kumaran Santhanam 2005-10-03 06:20:10 UTC
Version:            (using KDE KDE 3.4.2)
Installed from:    RedHat RPMs
OS:                Linux

There is an extension that I had come to love in Thunderbird
before switching to Kontact.  It is called QuickFile and allows
for rapid searching of mail folders by dynamic string match.

Since I couldn't live without, I decided to implement a similar
feature in Kmail.  I also implemented an extra hotkey action,
'J' for Jump to Folder, using the same dialog box.  To use the
dynamic search feature, simply type into the dialog box.  The
existing Move and Copy actions also invoke the enhanced dialog box.

Attached is a screenshot and patch against KMail 3.4.0.  I have
been using this patch for a number of months without any problems.
Recently, I upgraded to KMail 3.4.2 and verified that it still works.

I would love to see this feature included in the next minor release
of KDE.  I hope that other users can also find it to be a convenient
way to navigate their mail folders.
Comment 1 Kumaran Santhanam 2005-10-03 06:21:44 UTC
Created attachment 12823 [details]
Enhanced folder selection dialog
Comment 2 Kumaran Santhanam 2005-10-03 06:23:38 UTC
Created attachment 12824 [details]
Patch to implement enhanced folder selection feature

Applied and tested against KMail 3.4.0 and 3.4.2
Comment 3 Till Adam 2006-01-14 11:30:41 UTC
Heya Kumaran, I've looked at the patch and it seems like a good idea. Unfortunately it doesn't apply against 3.5 (and therefor trunk) anymore. Do you have time to port it to the changes in the selector dialog that happened since 3.5, maybe? Since 3.5 is feature frozen, this would go into 4.0, so we are in no real hurry. Distributions might pick it up for their 3.x based packages, so it's definitely worth porting. Please tell me if you don't have time, then we can try to find someone on the lists who might be willing to take this on. Thanks a lot for your contribution, and please excuse our lack of responsiveness, the team is way to small. :)
Comment 4 Kumaran Santhanam 2006-01-20 06:31:24 UTC
Created attachment 14312 [details]
Patch for version 3.5.0

Hi Till,

I've attached a patch which applies cleanly against 3.5.0.
The main functionality seems to work as expected.  I would
appreciate if the team could take a look and report any issues.

One difference from 3.4.2 is that the inactive rows are no
longer grayed out.  Please compare with the previous screenshot
attachment.  I wanted to resolve this issue, but unfortunately
ran out of time before tracking it down.  Perhaps someone on
the team would be willing to fix it?

Best Regards,
Kumaran
Comment 5 Till Adam 2006-01-21 12:52:44 UTC
On Friday 20 January 2006 06:31, Kumaran Santhanam wrote:

> I've attached a patch which applies cleanly against 3.5.0.
> The main functionality seems to work as expected.  I would
> appreciate if the team could take a look and report any issues.


Ok, I've applied it locally and will be testing it a bit over the next 
few days. I use 'c' and 'm' all day, so I'm sure I'll notice if 
problems come up. One thing I noticed, the old version could remember 
the last used folder in between invocations, which I found useful. 
Should not be hard to add that back in, right? Also, the initial width 
of the first column was very small, leading to an odd-looking initial 
dialog.
Comment 6 Kumaran Santhanam 2006-02-27 22:04:48 UTC
I hope that you've had a chance to use the feature over the last month.
Would it be possible to get it committed into the main trunk?  It might
help to avoid another round of porting in case the trunk becomes
unpatchable again.  Please feel free to let me know if I can be of
assistance.
Comment 7 Till Adam 2006-02-28 08:06:08 UTC
Yes, and I rather like it. I do think it would be nice to retain those features of our current folder selector like remembering the last used folder, etc. I'll have a look at the code later today, haven't looked at that at all yet, and then give you some more feedback, hopefully.
Comment 8 Kumaran Santhanam 2006-03-18 18:18:06 UTC
Is there any possibility of getting this feature into a point release like 3.5.2?
Comment 9 Till Adam 2006-03-18 18:20:54 UTC
I would like to see it go in and will discuss it with the others. We (KDE) are currently debating lifting the feature freeze a bit anyhow, for 3.5.3. 3.5.2 was just tagged yesterday, but was bugfixes only anyhow. I've fixed the remembering of the last used folder bug, btw, caused by the initial filter ("") automatically selecting the root.
Comment 10 Kumaran Santhanam 2006-03-18 18:33:05 UTC
Thanks for the advocacy and filter fix.

There is one other minor feature that I just remembered:

When the user types a search string and it displays the matched folders,
the arrow keys only allow selection of matched folders and not
unmatched parents.  When I implemented quick filing in 3.4, I was able
to gray out those non-selectable parent folders.  For some reason, the
same code did not gray them out in the 3.5 patch.

This is just a cosmetic issue, since the search results are fairly
obvious.  However, in the case of deep folder hierarchies, it might
make things more clear.

Do you have any ideas on how to implement this in 3.5?
Comment 11 Till Adam 2006-04-15 10:41:48 UTC
I've posted a request for inclusion in 3.5.3 to core-devel. We've slightly lifted the feature freeze, since 4.0 is so far off.
Comment 12 jos poortvliet 2006-04-15 16:10:35 UTC
i'm for inclusion :D
seems like it complies with requirements.
Comment 13 Till Adam 2006-05-01 20:28:14 UTC
SVN commit 536268 by tilladam:

Tiny feature that is part of the folder quick filing patch: Make
ctrl-j open the folder selector (with quick search) to jump to
a folder. I'll commit the rest of the patch in a bit.

Patch by Kumaran Santhanam.

CCMAIL: 113759@bugs.kde.org


 M  +17 -0     kmmainwidget.cpp  
 M  +1 -0      kmmainwidget.h  


--- branches/KDE/3.5/kdepim/kmail/kmmainwidget.cpp #536267:536268
@@ -580,10 +580,15 @@
   new KAction( i18n("Copy Message to Folder"), Key_C, this,
                SLOT(slotCopyMsg()), actionCollection(),
                "copy_message_to_folder" );
+  new KAction( i18n("Jump to Folder"), Key_J, this,
+               SLOT(slotJumpToFolder()), actionCollection(),
+               "jump_to_folder" );
   mAccel->connectItem(mAccel->insertItem(Key_M),
 		     this, SLOT(slotMoveMsg()) );
   mAccel->connectItem(mAccel->insertItem(Key_C),
 		     this, SLOT(slotCopyMsg()) );
+  mAccel->connectItem(mAccel->insertItem(Key_J),
+		     this, SLOT(slotJumpToFolder()) );
 
   // create list of folders
   mFolderTree = new KMFolderTree(this, folderParent, "folderTree");
@@ -1483,6 +1488,18 @@
 }
 
 //-----------------------------------------------------------------------------
+void KMMainWidget::slotJumpToFolder()
+{
+  KMail::KMFolderSelDlg dlg( this, i18n("Jump to Folder"), true );
+  KMFolder* dest;
+
+  if (!dlg.exec()) return;
+  if (!(dest = dlg.folder())) return;
+
+  slotSelectFolder( dest );
+}
+
+//-----------------------------------------------------------------------------
 void KMMainWidget::slotMoveMsg()
 {
   KMail::KMFolderSelDlg dlg( this, i18n("Move Message to Folder"), true );
--- branches/KDE/3.5/kdepim/kmail/kmmainwidget.h #536267:536268
@@ -282,6 +282,7 @@
   void slotSaveMsg();
   void slotOpenMsg();
   void slotSaveAttachments();
+  void slotJumpToFolder();
   void slotMoveMsg();
   //void slotMoveMsgToFolder( KMFolder *dest);
   void slotCopyMsgToFolder( KMFolder *dest);
Comment 14 Till Adam 2006-05-01 20:31:35 UTC
SVN commit 536272 by tilladam:

Patch by Kumaran Santhanam implementing folder quick filing as described in
http://bugs.kde.org/show_bug.cgi?id=113759. Ok'd on core-devel, reviewed,
tested, etc, as per the "lifted feature freeze" policy.

Thanks for the patch, mate.

BUG: 113759


 M  +192 -0    kmfolderseldlg.cpp  
 M  +7 -0      kmfolderseldlg.h  
Comment 15 Ingo Klöcker 2006-05-14 23:19:57 UTC
I'm sorry, but I'd like to see this patch removed again. Adding a second column to a dialog which is supposed to be a simple folder selection dialog is IMO not an option. Since the second column basically shows the same information as the first column there's obviously one column too much. Add to this that the second column takes basically all space which makes it necessary to make the folder selection dialog unreasonably wide (and which, on first invocation after updating, made me go "wtf happened to the dialog. where's the folder tree?"). We got many reports from users who thought the folder tree had vanished when we introduced the Unread/Read columns (which also took up basically all space). Moreover, the column layout is not saved/restored, so that the next time I move a message the new column again takes up all space.

Since a second column is not an option I suggest one of the following options:
- Make the quick filing work with the original layout, i.e. without adding the second column. Or, if really necessary, add the second column, but make it a hidden column.
- Remove the first column. This would make it possible to get rid of all folders which don't match the currently entered pattern even the parents of matching folders. (But such a change can't be done in a bug fix release.)

Since KDE 3.5.3 is supposed to be tagged in 9 days and QListView unfortunately doesn't provide an easy way to hide the second column and adding the current pattern to the first column header would violate the message freeze, I have to revert the patch for now. I don't take this decision easily, but I have no other choice.
Comment 16 Ingo Klöcker 2006-05-14 23:22:07 UTC
SVN commit 540881 by kloecker:

Revert the last change for now.
CCBUG: 113759

 M  +0 -192    kmfolderseldlg.cpp  
 M  +0 -7      kmfolderseldlg.h  
Comment 17 Kumaran Santhanam 2006-05-15 02:44:42 UTC
There is a very specific reason for the second column.  It was not
added frivolously to show redundant information.  Rather, it shows
the entire path to that folder in one line.  This is very important
while performing searches on deep hierarchies.

Take the following set of folders:

Parent              Parent
  Apples            Parent/Apples
    Oranges         Parent/Apples/Oranges
    Pears           Parent/Apples/Pears

Let's assume that the sub-folders of Parent are so numerous that the
entire hierarchy is not visible on a single page.  When the user types
'apple', it can be immediately seen why 'Pears' is visible.  Furthermore,
if the user types 'apples/oranges', those matched sub-folders will be
shown.  Without the second column, it is easy to get lost within the
hierarchy.
Comment 18 Kumaran Santhanam 2006-05-15 03:15:17 UTC
I would like to submit a comment describing the history of this patch
and some concerns that I have over the process of its evaluation.

This patch was initially developed last summer, almost one year ago.
Initially, the dialog did not have a second column, but I found that it
became necessary as folder hierarchies became more complex.  As such,
the column was added before the patch was submitted to the KDE bug
database.

I deployed the patch throughout our organization and it has been tested
very well by our users.  I waited until everything was stable before
submitting the patch into the KDE bug database.  As such, by the time
it was submitted, it was already indispensable to those of us who have
been using it every day.

I understand the resource constraints of the KDE team and was more than
happy to port the patch to the latest version in the hopes that it would
be included in the upcoming point release.  The port took some time, but
I was willing to help move the project along.  Many thanks to Till Adam
for getting it reviewed and committed into the source tree.

I am concerned that comment #15 was made at the last minute with only
a cursory evaluation, resulting in a reversion of the committed patch.
This patch has been in the KDE bug database since Oct 2005.  If the issues
mentioned in the comment were so critical, would it not have been better
to address them at an earlier date?

Here is a compromise which I hope would be acceptable for 3.5.3:

- In normal operation, the dialog has only one column with the second
  column being hidden from view.

- If the user types anything, the second column appears to show the
  full folder path and search terms.

- If the typed search terms are deleted using backspace, the second
  column once again disappears from view.

If this sounds good, please let me know as soon as possible so that I
have time to implement it before the tagging deadline.  I will not
be available next week, so it would be best to resolve everything by
this Friday if possible.
Comment 19 Ingo Klöcker 2006-05-15 23:15:35 UTC
As I said I'm sorry, but apart from the second column problem there was also the other problem I mentioned, namely that the very first time the dialog with two columns is invoked the second column will take all space. Of course, this is only an issue the first time the dialog is invoked because most users would resize the dialog to make the first column visible again. But nevertheless this small problem shows that the patch was not well enough tested. The first impression is important and therefore all new features (in particular if they involve resizable dialogs) have to be tested with new users and with upgrading users.

I fully understand why the second column was added, but it's the wrong solution for the problem. Two columns representing the same information is just wrong. If you reread my first comment you'll see that I proposed as one option to get rid of the first column and only show the second column. Of course, we'd lose the collapseblity of subfolder hierachies, but I wonder whether this is actually needed in a folder selection dialog whose only purpose is to allow the user to select a folder as easily and quickly as possible.

Yes, it would have been better to address the issues at an earlier date. Unfortunately, the last 6-12 months I had almost no time to do any KDE-related work. Hopefully this will change in the future. If I would have continued to have no time for another two weeks then your patch would have been in KDE 3.5.3. But, two days ago I finally had time again after month to compile the recent svn-branch-version of KMail. The first time I invoked the "Move Message" dialog I was shocked (as I already wrote above). If the dialog would have been opened with a reasonable default width I would probably have accepted the change. But that was not the case.

I propose the following compromise for 3.5.3:
- Get rid of the first column.
- Use the title of the first column, i.e. "Folder" as title of the second column (instead of "Path").
- Make sure the dialog is started with a reasonable default width, i.e. all paths of depth three should be completely visible, but the dialog must not be wider (by default) than 3/4 of the screen width.
- I'm a bit concerned about deep folder hierachies with long folder names because this would require the dialog to be very wide. Maybe path which don't fit into the current dialog width should be center-squeezed (?). See KStringHandler::cPixelSqueeze() (http://developer.kde.org/documentation/library/3.5-api/kdelibs-apidocs/kdecore/html/classKStringHandler.html#e19).

If you provide a patch implementing the first three points then I'll test it and consider re-adding your patch. Unfortunately, the time frame for inclusion in KDE 3.5.3 is rather short. I'd prefer to aim for inclusion in KDE 3.5.4 instead (with path squeezing).

BTW, looking at the patch I found the following word puzzle:
i18n("Path") + "  ( " + filter + " )"
Read http://developer.kde.org/documentation/library/kdeqt/kde3arch/kde-i18n-howto.html#SECTION00036000000000000000 if you want to learn how to do it correctly. (But for 3.5.3 you should leave it as it is except for replacing "Path" with "Folder".)
Comment 20 Kumaran Santhanam 2006-05-19 07:09:58 UTC
Thanks for the feedback.

I spent a significant amount time thinking about your suggestion
from both a user interface and coding perspective.  I also had a
chance to talk to other users about this to get a better sense of
the overall user experience.

I respectfully disagree with the proposal to eliminate the first
column.  The user feedback that I received indicates that the first
column provides a handy visual hierarchy reference.  The second
column is necessary to elucidate the matching algorithm.  They do
not show exactly the same information, but rather different
perspectives.  All of the users have been using the feature for
almost a year and thus have considerable experience with it.

I also wanted to mention that it seems a bit unfair to point at the
resize issue and make a claim that the code is not well-tested.  If
I had encountered this issue, I would have fixed it before
submitting the patch.  As it stands, we have found no problems with
the code after nearly a year of use.  The resize issue that you
encountered may be specific to a particular new or upgraded
configuration.  With that said, I'm more than happy to track down
and correct the issue that you experienced.

Removing the first column is a drastic change that could be met with
resistance from KDE users.  Furthermore, it requires quite a bit of
code surgery, due to the KFolderTree widget.  It would seem that the
safest approach is to resolve the resize issue and implement the
patch as-is, leaving the final decision to the KDE users.

As I had mentioned before, the patch has been in the database since
Oct 2005.  During that time period, since it had not been submitted,
I had to spend a significant amount of time upgrading it to the new
version of KMail.  The longer it stays out of the codebase, the more
maintenance it requires for new versions.

Given that user feedback has been good and the dialog is only
slightly altered, I ask that you consider accepting a version with
the resize issue resolved.  Then, it will be much easier to glean
ideas from the entire user base, resulting in more improvements
moving forward.
Comment 21 Kumaran Santhanam 2006-05-22 01:46:11 UTC
Created attachment 16210 [details]
Patch for version 3.5.2

Attached is a patch implementing the folder selection dialog with
the requested column width preservation functionality.	This patch
is against version 3.5.2.  There is a slightly different patch for
the version in SVN.
Comment 22 Kumaran Santhanam 2006-05-22 01:49:59 UTC
Created attachment 16211 [details]
Patch for version svn-20060521

This is the same patch in attachment 16210 [details] made against the
current SVN tree as of 2006-05-21.
Comment 23 Ingo Klöcker 2006-05-26 10:27:07 UTC
SVN commit 544826 by kloecker:

Re-add the folder quick filing patch now that the dialog size problems have been resolved. Patch by Kumaran Santhanam.
CCBUG: 113759

 M  +5 -0      kmail.upd  
 M  +214 -2    kmfolderseldlg.cpp  
 M  +11 -1     kmfolderseldlg.h  
 M  +2 -1      kmstartup.cpp  
Comment 24 Ingo Klöcker 2006-05-26 10:32:04 UTC
Unfortunately, I didn't see your new patch until now because the bug report wasn't cc'ed to kmail-devel. I'm sorry that your patch won't be in KDE 3.5.3, but it will be in KDE 3.5.4. Additionally to your patch I've made sure that the old stored value of "Size" is removed for upgraders, so that they also get the new wider default width.
Comment 25 Ingo Klöcker 2006-05-26 10:34:32 UTC
Till just encountered a crash in the folder selection dialog after typing something in, selecting and scrolling with the mouse wheel. We currently can reproduce it reliably. Did your users also encounter a crash in the dialog?
Comment 26 Ingo Klöcker 2006-05-26 10:47:42 UTC
If one adds and removes letters (i.e. first I enter "spa", then I press Backspace, then I type an "a" again) then more than one folder can be selected. Could you please look into this problem?
Comment 27 Kumaran Santhanam 2006-05-26 16:52:45 UTC
Comment 25:
We've never encountered a crash, and I have had no success in reproducing
the problem.  I'm using Fedora Core 4 with the packaged RPMs.  Can you
please give me some more steps to reproduce?

Comment 26:
This has not been reported before and I am unable to reproduce the problem.
It always seems to select one folder on my machine.  Perhaps the behavior
depends on a particular set of folders coupled with the search pattern.
Can you give me some more details about your setup?
Comment 28 Ingo Klöcker 2006-05-26 19:06:40 UTC
Comment 25: Only Till could reproduce the crash.

Comment 26: The following works for me: Folder hierarchy contains only the standard folders plus Local Folders/test/special and Local Folders/test/x-spam. Now select a message and press 'm'. Click with the mouse on the first line (Local Folders), type "spa" (and note that x-spam is selected, but Local Folders is still the current item), now press Backspace. Result: special and x-spam are both selected. If you press Backspace again, then inbox, special and x-spam are selected.
The problem does not occur if one selects the first line with the keyboard (e.g. by pressing the Home key). So there seems to be a difference between selection by mouse and selection by keyboard.