ddablib / envvars Goto Github PK

The component has been deprecated for years. Time to say goodbye to it.

Providing a lot of backward compatibility for older compilers is a lot of work for diminishing returns.

Also, I can't test compilers before XE.

The component provides the Values[] property that is indexed by env var name. Although TPJEnvironmentVars has get and set methods that provide the same functionality, maybe a similar class property that provides a familiar interface would be useful.

Additionally, would a read only Names[] class property (with integer index) be useful too?

I really don't like how messy the TPJEnvironmentVars.CreateBlock method is.

It would be much better to encapsulate the whole messy business in a separate class and provide a Memory property that points to the required block.

TPJEnvironmentVars.CreateBlock could then be overridden to accept this new object as a parameter if it was thought necessary to retain such a method. The old TPJEnvironmentVars.CreateBlock could then be deprecated.

Although the Data parameter of the various TPJEnvironmentVars enumerator methods is useful on occasion, a lot of the time it isn't required.

So how about a couple more enumerator overloads without the Data parameter? E.g.:

TPJEnvironmentVars = class(TObject)
// ...
public 
  // ...
  type
    TEnumNamesCallback = reference to procedure (const VarName: string);
    TEnumVarsCallback = reference to procedure(const EnvVar: TPJEnvironmentVar);
    //.. 
  class procedure EnumNames(Callback: TEnumNamesCallback); overload;
  class procedure EnumVars(Callback: TEnumVarsCallback); overload;
  // ...
end;

// ...

class procedure TPJEnvironmentVars.EnumNames(Callback: TEnumNamesCallback)
begin
  Assert(Assigned(Callback));
  EnumNames<Integer>(
    procedure (const VarName: string; Data: Integer)
    begin
      Callback(VarName);
    end,
    0 // Dummy value
  );
end;

class procedure TPJEnvironmentVars.EnumVars(Callback: TEnumVarsCallback)
begin
  Assert(Assigned(Callback));
  EnumVars<Integer>(
    procedure (const EnvVar: TPJEnvironmentVar; Data: Integer)
    begin
      Callback(EnvVar);
    end,
    0 // Dummy value
  );
end;

Like the component mentioned in issue #1, there are several deprecated functions in the unit that it's really time to remove

There's no need for the user to change any fields of TPJEnvironmentVar, so we can make the record invariant by hiding the fields, adding read only properties and providing a constructor:

TPJEnvironmentVar = record
strict private
  var
    fName: string;
    fValue: string;
public
  constructor Create(const AName, AValue: string);
  ///  <summary>Environment variable name.</summary>
  property Name: string read fName;
  ///  <summary>Environment variable value.</summary>
  property Value: string read fValue;
end;

The constructor will have have the obvious simple implementation.

Wherever the records fields are directly set in the code the constructor will need to be called instead.

Mapping and selection methods are often useful.

The mapping would transform each environment variable to some type T and create an array of T the same size as there are environment variables.

The selection method would select 0 or many environment variables and return an array containing them.

Both would use callbacks that are passed the environment variable and possibly the index and some data passed in by the user.

The mapping callback will return a value of the required type while the selection will return Boolean true to select the environment variable and false to exclude it.

Unicode environment blocks are distinguished when calling CreateProcess by passing the CREATE_UNICODE_ENVIRONMENT flag.

This raises the question of whether we can rely upon a process' environment block being Unicode? All processes that I've seen launched from modern Windows have Unicode environment blocks, but what about child processes explicitly launched with ANSI environment blocks? Does Windows convert them to Unicode before the process is started?

This is very important to know because the TPJEnvironmentVars.GetAll method assumes that its host process has a Unicode environment block and will fail if the block is ANSI. Therefore TPJEnvironmentVars.GetAll will fail on any process that has an ANSI environment block. That would be a major bug.

Given that Windows knows an environment block is Unicode because of the CREATE_UNICODE_ENVIRONMENT flag, is there a way testing for that flag? Note that this is only important if Windows doesn't convert ANSI environment blocks to Unicode.

Tests are required.

Originally posted by @delphidabbler in #12 (comment)

Changing the API breaks the demo programs. Furthermore the demos have no demos of the new features.

At present there are two enumerator methods of TPJEnvironmentVars:

class procedure EnumNames(Callback: TPJEnvVarsEnum; Data: Pointer);
class procedure EnumVars(Callback: TPJEnvVarsEnumEx; Data: Pointer);

As can be seen these enumerators are passed data in the traditional way, as a pointer. Often the pointer would be cast to an object on an integer, which isn't type safe.

How about adding some templated overloaded methods like these:

class procedure EnumNames<T>(Callback: TPJEnvVarsEnum<T>; Data: T);
class procedure EnumVars<T>(Callback: TPJEnvVarsEnumEx<T>; Data: T);

We will need new enumerator types:

TPJEnvVarsEnum<T> =  reference to procedure(const VarName: string; Data: T);
TPJEnvVarsEnum<T> =  reference to procedure(const EnvVar: TPJEnvironmentVar; Data: T);

The original methods and callback types could be deprecated.

The TPJEnvironmentVars.GetAll methods return both TStrings and TArray<TPJEnvironmentVar> types.

I'm suggesting the following additional overloads:

• GetAll(const List: TList<TPJEnvironmentVar>)
• GetAll(const D: TDictionary<string,string>)

The TPJEnvironmentVars.GetAll method parses the current environment block. There is also similar code in demo1 that parses a generated block.

Between the two there's enough code to create a suitable method. TPJEnvironmentVars.GetAll could then be rewritten to use the new method to parse the current environment block.

This check considers =Value as a valid environment variable, but it isn't.

Change to use TPJEnvironmentVar.IsValidStr instead.

Once the component is removed (issue #1) the enumerator callback type definitions apply only to TPJEnvironmentVars. It would therefore make sense to move them inside the class. We could also take the opportunity to rename them. E.g.

type
  TPJEnvironmentVars = class(TObject)
  // ...
  public
    type
      TEnumNamesCallback = reference to procedure(const VarName: string; Data: Pointer);
      TEnumVarsCallback = reference to procedure(const EnvVar: TPJEnvironmentVar; Data: Pointer);
    // ...
    class procedure EnumNames(Callback: TEnumNamesCallback; Data: Pointer);
    class procedure EnumVars(Callback: TEnumVarsCallback; Data: Pointer);
  end;

The old TPJEnvVarsEnum and TPJEnvVarsEnumEx types could either be removed or be deprecated and redefined as follows (after TPJEnvironmentVars):

type
  TPJEnvVarsEnum = TPJEnvironmentVars.TEnumNamesCallback deprecated;
  TPJEnvVarsEnumEx = TPJEnvironmentVars.TEnumVarsCallback deprecated;

Although TPJEnvVarsEnumerator can be used stand alone, its real purpose is to provide an enumerator for the component. If the component is removed (issue #1) then there is no real need to keep this enumerator class.

Given that TPJEnvironmentVars provides various enumerator methods there is no real need to use TPJEnvVarsEnumerator as a stand alone enumerator, especially since the TPJEnvironmentVars methods are easier to use.

Therefore TPJEnvVarsEnumerator could be deleted, given that it is not currently deprecated, it could be retained but deprecated.

A common thing to do us to render an environment variable in name=value format. Add a ToString method to do this.

It is useful (eg when creating environment blocks) to know the size in chars of an environment variable in name=value format. So add a Size method to return the size in characters. (The size in bytes will be SizeOf(Char) * Size).

It could be useful to be able to compare TPJEnvironmentVar records.

Need to decide how to compare. Do we consider two records to be equal if their names are the same or if both name and value are the same.

Tests need to be case insensitive because Windows considers environment variable names to be the same if they differ only in the case of the letters.

Some code in the unit validated an environment variable. This functionality really belongs in TPJEnvironmentVar.

Presently the Expand() button expands the value in edName, and writes it into the edValues edit control.

This doesn't seem logical. Wouldn't it be better to expand the value in edValues and display it somewhere else (maybe the list box)?

Rename units as:

• DelphiDabbler.Lib.EnvVars
• DelphiDabbler.Lib.EnvVarsDsgn

.dcr file will need renaming similarly.

The will be a breaking change so needs introducing at the next major version release.

TStringDynArray can be replaced with TArray<string> or array of string as appropriate.

Similarly TPJEnvironmentVarArray can be replaced by TArray<TPJEnvironmentVar> or array of TPJEnvironmentVar.

The definition of the TPJEnvironmentVarArray type can either be deleted or deprecated.

This exception class was only used by the component.