Drive Toolbar Improvements

Discuss development issues and submit patches here
ajs
Posts: 409
Joined: Mon Jul 05, 2010 6:37 pm

Re: Drive Toolbar Improvements

Post by ajs »

twinsen wrote:On another computer I had 13 drives, which made the drive labels take up a bit too much room.
I might try it to with just DVDDrives and usb sticks (removable media), since they are the ones where the label means more than the letter.
Also maybe the ability to hide some drives I never use. There are some network drives and of course the a:\ that I don't care about.
you could an entryin the popup menu "show/hide drive label". Then the general option is the default and each drive could be customized.
Same for the possibility to hide drives: an entry in the popup menu to remove it and a dialog to show again hidden drives.

twinsen
Posts: 109
Joined: Mon Dec 27, 2010 3:17 pm

Re: Drive Toolbar Improvements

Post by twinsen »

Installing TortoiseSVN 1.6.15 fixed the patch errors for the rc file.
Although I can't reboot at the moment to properly integrate it into explorer, running "TortoiseMerge" from the start menu works fine.
Just select "Apply unified diff", the diff file and directory to patch.

ajs
Posts: 409
Joined: Mon Jul 05, 2010 6:37 pm

Re: Drive Toolbar Improvements

Post by ajs »

After developing patch 26 against commit 163, merging it to patch 06A makes some problems.
So I have reviewed the code of patch 06A and re-worked that.
The changes are:
1) re-arrangement of the code for the address bar filter
2) removed the "DirKeepFilter" property and functions. Removin the filter when changing directory should be an option customizable by the user, so I am planning to reintroduce that in a future patch

Here is the new patch against commit 163. The old patch has been removed.

There is still some little things to fix for the refresh (I have already did that in my local development branch, but need to create a patch).
Attachments
patch_06A__v163.7z
(1.18 KiB) Downloaded 376 times

ajs
Posts: 409
Joined: Mon Jul 05, 2010 6:37 pm

Re: Drive Toolbar Improvements

Post by ajs »

ajs wrote:There is still some little things to fix for the refresh (I have already did that in my local development branch, but need to create a patch).
This patch fixes the refresh issue mentioned above.
The patch is against commit 163 + patch 06A + patch 26. The right way to apply the patch is:
1) apply patch 06A to commit 163 (http://www.explorerplusplus.com/forum/v ... t=10#p1891)
2) apply patch 26 to the result of 1) (http://www.explorerplusplus.com/forum/v ... ?f=7&t=660)
3) apply this patch
Attachments
patch_27__v163.p06A.p26.7z
(992 Bytes) Downloaded 327 times

ajs
Posts: 409
Joined: Mon Jul 05, 2010 6:37 pm

Re: Drive Toolbar Improvements

Post by ajs »

I had to re-work patch 06B against commit 163 since I need this as base for another patch.
Here is the updated file (before the patch was against commit 159)
Attachments
patch_06B__v163.7z
(6.41 KiB) Downloaded 331 times

David Erceg
Site Admin
Posts: 909
Joined: Sat Apr 18, 2009 1:46 am

Re: Drive Toolbar Improvements

Post by David Erceg »

A few notes on patch 6A:

Code: Select all

for(i=0; i < lstrlen(szPath); i++)
{
	if(szPath[i] == '*')
	{
		bFound = TRUE;
		break;
	}
}
I know I've probably done exactly this in places (hopefully only in the older parts of the code!), but use the standard library.

Code: Select all

for(i=lstrlen(szPath)-1; i >= 0; i--)
{
	if(szPath[i] == '\\')
	{
		bFound = TRUE;
		break;
	}
}
See PathStripPath as well as PathRemoveFileSpec. They make it easier to manipulate a path without manually parsing it.

Code: Select all

if(bFound)
{
	StringCchCopy(szPathWithoutFilter,i+1,szPath);
	StringCchCopy(szFilter,SIZEOF_ARRAY(szFilter),&szPath[i+1]);

	// Remove the filter from the path, so the path is valid.
	StringCchCopy(szPath,SIZEOF_ARRAY(szPath),szPathWithoutFilter);

	SendMessage(m_hAddressBar,WM_SETTEXT,0,(LPARAM)szPath);
}
Similar to above, however the last line (WM_SETTEXT) isn't required. Once the directory has been browsed to, the address bar text will be set anyway.

Finally, the tab contents should only be filtered after the directory has changed. It could also check if the directory entered matches the current directory, and only filter (instead of browse) if it is.

David Erceg
Site Admin
Posts: 909
Joined: Sat Apr 18, 2009 1:46 am

Re: Drive Toolbar Improvements

Post by David Erceg »

I haven't read over all of patch 6B yet, but I don't think testing if a drive is a floppy drive using its type name is reliable (or even a good way, sorry!). See the second paragraph of my post here for a better way of implementing the functionality. As far as things like the drive toolbar go, a user option to hide certain drives permanently may work.

Unfortunately, I won't be committing any part of the patch that specifically tries to deal with floppy disks.

David Erceg
Site Admin
Posts: 909
Joined: Sat Apr 18, 2009 1:46 am

Re: Drive Toolbar Improvements

Post by David Erceg »

Within 6B:

Within OptionsDialog.cpp, only call DrivesToolbarRefreshAllIcons() if the setting actually changed.

Code: Select all

std::wstringstream sDisplayName;
sDisplayName << szDisplayName << TEXT(" (") << szVolume << TEXT(")");
std::wstring wDisplayName = sDisplayName.str();
size_t tSize = (wDisplayName.length() + 1) * sizeof(TCHAR);
memcpy(szDisplayName, wDisplayName.c_str(), tSize);
Far too complicated for what it accomplishes. The same thing could be done simply as:

Code: Select all

StringCchPrintf(szDisplayName,SIZEOF_ARRAY(szDisplayName),_T("%s (%s)"),szDisplayName,szVolume);
Overall, I'm all for greater use of C++ within the codebase, but in the interests of keeping things consistent, I think C should be preferred in some of the older files, such as DrivesToolbarHandler.cpp. This is especially true when the equivalent C code would be simpler and shorter than the C++ code.

Also, a large chunk of the code within InsertDriveIntoDrivesToolbar() is the same as the code in InsertDriveIntoDrivesToolbar(). Find a way to extract the common functionality.

Finally, I feel I should say the comments in this post and the two above are not meant to be critical. They're more my blunt assessment of the code. I know I've probably done exactly the same types of things as above in portions of the code, but you have to keep in mind that Explorer++ was started in 2005, and parts of the code are quite old.

My views on it are:
1. I impose stricter standards on myself now than I once did. That either means I try not to commit "bad" code, or I refactor it shortly afterwards.

2. I impose even stricter standards on code submitted by other people. Mainly because I'm not sure if they'll be around to refactor it; in all honesty, I'll probably be the person that has to maintain the code, which means I want it to be as good as possible when its first committed.

Nonetheless, thanks for the work you've both done so far Michael and Ryan, it is appreciated.

ajs
Posts: 409
Joined: Mon Jul 05, 2010 6:37 pm

Re: Drive Toolbar Improvements

Post by ajs »

I will review the patch following your last suggestions.

ajs
Posts: 409
Joined: Mon Jul 05, 2010 6:37 pm

Re: Drive Toolbar Improvements

Post by ajs »

I have rework patch 06A following David's suggestions

Patch against commit 186
Attachments
patch_06A__v186.7z
(1.16 KiB) Downloaded 333 times

Post Reply