Nppcommunity npp-community → v1.0.0-rc2

Comments and changes to this ticket

• 

  Thell Fowler October 8th, 2009 @ 03:33 PM

  A msdn forum thread has been opened on this issue at:

  http://social.msdn.microsoft.com/Forums/en-US/vcprerelease/thread/0...

• 

  Thell Fowler October 8th, 2009 @ 03:35 PM

  • Milestone set to “v1.0.0-rc1”
• 

  Jocelyn Legault October 8th, 2009 @ 10:26 PM

  • Title changed from “Unit Test Failures on Win7” to “[Bug] Unit Test Failures on Win7”
• 

  Thell Fowler October 16th, 2009 @ 06:51 PM

  • Milestone changed from “v1.0.0-rc1” to “v1.0.0”
• 

  Thell Fowler October 16th, 2009 @ 06:58 PM

  • Milestone changed from “v1.0.0” to “v1.0.0-rc2”
• 

  Dave Brotherstone October 18th, 2009 @ 09:07 PM

  • State changed from “new” to “open”

  For the PathRemoveFileSpec failure, that is simply an improvement on their algorithm:
  They're now handling c:\abc\def in PathRemoveFileSpec more sensibly, to become c:\abc rather than c:\abc\

  For PathAppend, this is mainly due to a very strange change in behaviour in Win7, to do with PathCanonicalize.

  ..\abc\def = abc\def
  

  • explicitly NOT what the docs say, it should be \abc\def, but, following the logic that if it starts with ..\, the \ is removed, we accept that change and continue...

  \..\abc\def = \abc\def
  

  • ok, this is the same as the previous version

  ..\abc\..\def = \def
  

  • Pardon? ..\abc\def = abc\def, but ..\abc\..\def = \def, what on earth?
  • if we re-wrote the path to be ..\def, which would be the same directory (relatively), it would return 'def', so a .. in the middle causes that rule to break

  ..\abc\def\..\ghi = abc\ghi
  

  • but alas, that isn't always the case!

  Hence, this is the logic that must be mirrored:
  

  ..\abc\def           abc\def
  ..\abc\..\def        \def
  ..\abc\def\..\ghi    abc\ghi
  

  The generic_string version of PathCanonicalize needs to be re-developed using a list of path-sections, and then it can process the path from the start instead of the end, which will make the algorithm significantly simpler, albeit less efficient.
  However, as can be seen from this table, the logic of whether the leading backslash is present is very odd.

• 

  Jocelyn Legault October 18th, 2009 @ 09:31 PM

  ..\abc..\def = \def That makes sense IMO

  if ..\abc becomes abc and ..\abc\def = \abc\def, then [..\abc][..\def] =>
  [..\def] and [..\def] => \def

• 

  Dave Brotherstone October 18th, 2009 @ 10:12 PM

  Logic identified, I think, as if the "root" is ever touched, ignoring any leading .., then a backslash is prepended to the output.

  ..\abc\..\def

  1. ignore initial ..
     
  2. abc, now in directory abc
     
  3. .., now at root, prepend \ to output
     
  4. def, now at \def.
     
  5. output result - \def

  Requesting a vote on this:

  1. Copy the generic_string's into TCHARs, call the win32 function and copy back
     or
     
  2. Reimplement and do the windows version check for the edge case
• 

  Jocelyn Legault October 18th, 2009 @ 11:10 PM

  My vote goes strongly in favor of 1 for the following reasons:

  • We will be assured that we will have an identical behaviour as the original Win32 one, no matter which platform / compiler we use.
    
  • This function is never used in tight loops where realtime performance is needed.

  So, while it'd be nice to have a "native" std::string implementation, it
  think the better solution is the simplest in this case.

• 

  Dave Brotherstone October 20th, 2009 @ 07:52 PM

  • State changed from “open” to “needs_ack”
  • Assigned user changed from “Dave Brotherstone” to “npp-community”

  (from [250a8d32fb4d613b02b5fca91826ef25bc663097]) Common.cpp: PathXXX functions on Windows 7 fix

  • PathCanonicalize has been made to simply call the Win32 function, as the difference between Win7 and prior versions is too great
  • PathRemoveFileSpec added some extra logic in for double backslashes in the middle of a path
  • Additional debug output added to tests to aid in debugging tests if they fail

  [#4 state:needs_ack responsible:npp-community]

  Signed-off-by: Dave Brotherstone davegb@pobox.com
  http://github.com/davegb3/npp-community/commit/250a8d32fb4d613b02b5...

• 

  Thell Fowler October 20th, 2009 @ 08:10 PM

  • State changed from “needs_ack” to “acked”

  Ack. Will get on pu soon.

• 

  npp-community October 21st, 2009 @ 05:19 PM

  • State changed from “acked” to “needs_ack”

  (from [2a4b25270681344fb1162d0b0a8452b61e429007]) Common.cpp: PathXXX functions on Windows 7 fix

  • PathCanonicalize has been made to simply call the Win32 function, as the difference between Win7 and prior versions is too great
  • PathRemoveFileSpec added some extra logic in for double backslashes in the middle of a path
  • Additional debug output added to tests to aid in debugging tests if they fail

  [#4 state:needs_ack responsible:npp-community]

  Signed-off-by: Dave Brotherstone davegb@pobox.com
  http://github.com/npp-community/npp-community/commit/2a4b2527068134...

• 

  Thell Fowler October 21st, 2009 @ 05:19 PM

  • State changed from “needs_ack” to “proposed”

  (from [5abf9740311f688a2a1e3d5369615fe60f30955f]) Merge branch 'db/LH-4/PathFuncFix' into pu

  • db/LH-4/PathFuncFix: Common.cpp: PathXXX functions on Windows 7 fix

  [#4 state:proposed]

  Signed-off-by: Thell Fowler git@tbfowler.name
  http://github.com/npp-community/npp-community/commit/5abf9740311f68...

• 

  Thell Fowler October 21st, 2009 @ 05:37 PM

  Thanks Dave!

  I can confirm Win7 now passes all tests, I haven't tried this build on XP yet to verify it still passes all tests.

• 

  Thell Fowler October 26th, 2009 @ 04:46 PM

  • State changed from “proposed” to “candidate”

  (from [67d226aa8999e6cb2db9fc618912af20ca2d57d2]) Merge branch 'db/LH-4/fix-win7-path-handling' into next

  • db/LH-4/fix-win7-path-handling: Common.cpp: PathXXX functions on Windows 7 fix

  [#4 state:candidate]

  Signed-off-by: Thell Fowler git@tbfowler.name
  http://github.com/npp-community/npp-community/commit/67d226aa8999e6...

• 

  Thell Fowler November 29th, 2009 @ 06:34 AM

  • State changed from “candidate” to “resolved”

  (from [71c6fddd0f5457ad1dcbf834914ccf52b7ac4c43]) Merge branch 'db/LH-4/fix-win7-path-handling'

  • db/LH-4/fix-win7-path-handling: Common.cpp: PathXXX functions on Windows 7 fix

  [#4 state:resolved]

  Signed-off-by: Thell Fowler git@tbfowler.name
  http://github.com/npp-community/npp-community/commit/71c6fddd0f5457...