Code review comment for lp:~spud/spud/alias

Revision history for this message
Adam Candy (asc) wrote :

This looks good and adds some pretty helpful functionality.

Just a few points:

1. It would be good to have a description of this usage from diamond -h - i.e. that the argument to -s is a path or alias.

2. Blank lines in the .diamond/schemata/flml file are not skipped, leading to erroneous 'Warning: not a valid path: ' errors (with a blank path).

3. Currently the invalid path warning, e.g.
      Warning: not a valid path: /home/asc/cur/pgnonlinear/schemas/fluidity_options.rng
  occurs when any of the paths do not exist.
  It might be better to restrict this behaviour to when the -v verbosity switch is supplied,
  OR in the case where the missing path is actually the schema file diamond requires (well, in this case you see the error, 'Could not find alias ...' - this is enough).
  I have schemas listed that do not exist on all machines I'm working on.

And finally, just a suggestion:

The alias could be supplied without -s. Any argument with a dot suffix is a file to be loaded. An argument that does not contain a dot could be considered an alias.

review: Needs Fixing

« Back to merge proposal