Re: user defined properties

John Norris wrote:
> I have added code to allow user defined property types to be displayed in
> the drop down list.
> These property names are read from two files, one holding properties for
> files and the other for directories (or folders).
> If multiple files and / or directories are selected then properties for both
> are read and displayed.
> The path and name of the property files are stored in the registry.
> I have updated the settings dialog to add a new page "User defined
> properties". This allows the user to browse and enter the name for both
> "file property" and "directory property" files.
> These files must be text files in normal ASCII. I have not tested them with
> Unicode.
> The file names are stored in the registry under
> "Software\TortoiseSVN\FilePropertyFile" and
> "Software\TortoiseSVN\DirPropertyFile".
> When using the browse button for settings, if an entry already exists, the
> dialog will open the entry's directory.
> I have not updated the help files.

Thanks a lot for the patch. But I have some comments if you don't mind:

* please provide a patch for the HEAD of trunk, your patch is way
outdated and hard to apply that way.
* Don't leave OutputDebugString() statements in the code. If you really
want the left in, then use #ifdef DEBUG to exclude them from the release
build

Other comments inline:

Index: src/TortoiseProc/EditPropertyValueDlg.cpp
===================================================================
--- src/TortoiseProc/EditPropertyValueDlg.cpp (revision 7655)
+++ src/TortoiseProc/EditPropertyValueDlg.cpp (working copy)
@@ -16,6 +16,7 @@
  // along with this program; if not, write to the Free Software
  // Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
02111-1307, USA.
  //
+#include "windows.h"
  #include "stdafx.h"

That's already include in stdafx.h
btw: *everything* before the line
#include "stdafx.h" is *ignored* by the compiler!

  #include "TortoiseProc.h"
  #include "SVNProperties.h"
@@ -61,6 +62,8 @@
  BOOL CEditPropertyValueDlg::OnInitDialog()
  {
          CResizableStandAloneDialog::OnInitDialog();
+ struct pnode pn1, *ptr, *qptr;
         ^^^^^^
You have defined that 'pnode' is a struct type in the header, why
specify this here again?

+// if ((!m_bFolder)&&(!m_bMultiple))
+ if ((!m_bFolder)||(m_bMultiple))

Don't comment lines, just remove them! We use a version control system
after all and can recreate those if needed all the time.

+ {
+ m_regFPPath = CRegString(_T("Software\\TortoiseSVN\\FilePropertyFile"));
+ if (read_property_file(m_regFPPath, &pn1) == 0)

I would suggest to just store and read the file from
%APPDATA%\TortoiseSVN\CustomProperties
(use SHGetSpecialFolderPath or some other function to get the path to
%APPDATA%).

+
+// if (m_bFolder)
+// {

Again, just remove the lines.

+ lenW = ::MultiByteToWideChar(CP_ACP, 0, w1, i, 0, 0);

Why don't you just use a function in the CUnicodeUtils class?

+IMPLEMENT_DYNAMIC(CSettingsProperties, CPropertyPage)
+CSettingsProperties::CSettingsProperties()
+ : CPropertyPage(CSettingsProperties::IDD)
+ , m_sFPPath(_T(""))
+ , m_sDPPath(_T(""))

It's not necessary to initialize CString's with an empty string - the
default constructor already does that.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: users-help@tortoisesvn.tigris.org