Faculty of Information Technology
Software Engineering Group

Opened 9 months ago

Last modified 9 months 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)

Change History (2)

comment:1 Changed 9 months 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.