Faculty of Information Technology
Software Engineering Group

Opened 4 years ago

Last modified 4 years ago

#258 new todo

Review new implementation of `Win32OperatingSystem >> pathOfCommand:`

Reported by: Jan Vrany Owned by:
Priority: major Milestone:
Component: default Keywords:
Cc: Also affects CVS HEAD (eXept version): no

Description

I have rewritten Win32OperatingSystem >> pathOfCommand: but I'm not much
of a Windows expert. This patch should be reviewed.

Patrik, can you have a look?

Attachments (1)

libbasic_fix_1_of_1_rev_1c6f89bd0de6_Win32__fix__Win32OperatingSystem_class____pathOfCommand__.patch​ (8.5 KB) - added by Jan Vrany 4 years ago.

Download all attachments as: .zip

Change History (2)

Changed 4 years ago by Jan Vrany

Attachment: libbasic_fix_1_of_1_rev_1c6f89bd0de6_Win32__fix__Win32OperatingSystem_class____pathOfCommand__.patch​ added

comment:1 Changed 4 years ago by Patrik Svestka

I have taken a look. It is better than the previous implementations.

Here are my comments (of the patch):

  • on line 3963:

search := Array with: Filename currentDirectory.

perhaps as it grows with the information from PATH (which could be rather large) and the HKEY_CURRENT_USER\Environment the Array should be OrderedCollection:

search := OrderedCollection with: Filename currentDirectory.

  • on line 8349 (typo in comment):

aboce should be above

  • minor on line 4000 (superfluous spaces):

OperatingSystem pathOfCommand:'cvs'

Other than these the code looks fine to me.

Note: See TracTickets for help on using tickets.