Merge lp:~spud/spud/alias into lp:spud

Proposed by Fraser Waters
Status: Superseded
Proposed branch: lp:~spud/spud/alias
Merge into: lp:spud
Prerequisite: lp:~spud/spud/slice-view
Diff against target: 208 lines (+79/-50) (has conflicts)
3 files modified
diamond/bin/diamond (+46/-29)
diamond/diamond/config.py (+28/-17)
doc/spud_manual.tex (+5/-4)
Text conflict in diamond/diamond/config.py
To merge this branch: bzr merge lp:~spud/spud/alias
Reviewer Review Type Date Requested Status
Adam Candy Needs Fixing
Florian Rathgeber Needs Fixing
Review via email: mp+69092@code.launchpad.net

This proposal has been superseded by a proposal from 2011-08-12.

Description of the change

Allows schemata files to list key value pairs of alias name and xml path.

To post a comment you must log in.
Revision history for this message
Patrick Farrell (pefarrell) wrote :

Can you give an example of the new format in the manual, too?

Revision history for this message
Patrick Farrell (pefarrell) wrote :

Adam, can you please try out this branch and see if it satisfies your blueprint?

I don't like the idea of putting things into a schema_alias. It really doesn't fit with the whole system; but the first half of your proposal is useful.

Revision history for this message
Florian Rathgeber (florian-rathgeber) wrote :

Unfortunately the check in diamond:29 is not robust. The schema file will seldom be only alphabetic: if it's an actual path to a schema file, it will never be and even if it's an alias it might contain - or _ (unless this is explicitly disallowed). Any characters that are a valid branch name should be allowed as aliases.

review: Needs Fixing
Revision history for this message
Florian Rathgeber (florian-rathgeber) wrote :

Seems the case when it's an actual path to a schema file is not supposed to satisfy the if in diamond:29. However, any one non-alpha characters in the alias causes it to be treated as a path, which does obviously not exist. I suggest all alphanumeric characters and - and _ should be allowed in an alias.

Revision history for this message
Fraser Waters (fraser-waters08) wrote :

> Seems the case when it's an actual path to a schema file is not supposed to
> satisfy the if in diamond:29. However, any one non-alpha characters in the
> alias causes it to be treated as a path, which does obviously not exist. I
> suggest all alphanumeric characters and - and _ should be allowed in an alias.

Correct. I'll change this to allow those characters.

lp:~spud/spud/alias updated
443. By Fraser Waters

Added '-' and '_' as valid alias characters.

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
Revision history for this message
Stephan Kramer (s-kramer) wrote :

My 2 cents:
Could we not make the alias use a different option, say -a. So we don't need to do any interpreting of arguments to see whether they're an alias for a schema, a path to a schema, or an .xml options file. I'd like to stick to conventions that filenames can have arbitrary (os allowed) names.

Revision history for this message
Fraser Waters (fraser-waters08) wrote :

@asc
1. Yes that help will be added once this is finalized.
2. Will fix to ignore blank lines.
3. Will fix to only print under verbose.
@s-kramer
I'm assuming -s and -a would be mutually exclusive? If you've already given a schema to load an alias means nothing. If they're both given should one override the other or is that an error?

Revision history for this message
Florian Rathgeber (florian-rathgeber) wrote :

I guess -s would override -a if in doubt. But since this is an assumption that is not clear to the user I think raising an error is preferred.

I agree with s-kramer that any "legal" file name should be allowed.

Just a thought I had the other day: bzr branches (can) have a nickname you can read and set with `bzr nick` (it is automatically set if you `bzr branch`). In case neither -a nor -s are given, diamond could try using the bzr nick to auto-select the alias. In case bzr nick is not available (i.e. you're not running diamond from within a bzr branch) it could go back to the defaults.

Revision history for this message
Fraser Waters (fraser-waters08) wrote :

> I guess -s would override -a if in doubt. But since this is an assumption that
> is not clear to the user I think raising an error is preferred.

What about pointing it out in the help and raising a warning if both are set. I'm just wondering if people might shortcut their diamond to use one alias normally and then add -s sometimes to load a file? Is that a valid use case?

> I agree with s-kramer that any "legal" file name should be allowed.

Yes in hind sight it wasn't a good idea to try and differentiate on one option.

> Just a thought I had the other day: bzr branches (can) have a nickname you can
> read and set with `bzr nick` (it is automatically set if you `bzr branch`). In
> case neither -a nor -s are given, diamond could try using the bzr nick to
> auto-select the alias. In case bzr nick is not available (i.e. you're not
> running diamond from within a bzr branch) it could go back to the defaults.

If others agree I'm sure it can be put in.

Revision history for this message
Patrick Farrell (pefarrell) wrote :

I think -s and -a should be mutually exclusive (i.e. error if both are set).

I don't think diamond should have any bzr-specific code.

lp:~spud/spud/alias updated
444. By Fraser Waters

Added -a option

445. By Fraser Waters

Merge from trunk#

446. By Fraser Waters

Updated manual

447. By Fraser Waters

Missed a colon before the -s command line option

448. By Fraser Waters

Merge from trunk

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'diamond/bin/diamond'
--- diamond/bin/diamond 2011-07-05 11:42:44 +0000
+++ diamond/bin/diamond 2011-08-12 09:02:24 +0000
@@ -20,6 +20,7 @@
20import os.path20import os.path
21import sys21import sys
22import traceback22import traceback
23import string
2324
24import gtk25import gtk
25import gtk.gdk26import gtk.gdk
@@ -47,6 +48,7 @@
47 "-h Display this message\n" + \48 "-h Display this message\n" + \
48 "-f Forks at startup\n" + \49 "-f Forks at startup\n" + \
49 "-s [SCHEMAFILE] Use the supplied schema file *\n" + \50 "-s [SCHEMAFILE] Use the supplied schema file *\n" + \
51 "-a [ALIAS] Use the supplied alias, if not given use \"default\"\n" + \
50 "-t [TRONFILE] Use the supplied schematron file for extended validation\n" + \52 "-t [TRONFILE] Use the supplied schematron file for extended validation\n" + \
51 "-v Verbosity switch - if supplied Diamond prints additional\n" + \53 "-v Verbosity switch - if supplied Diamond prints additional\n" + \
52 " debugging information to standard output and standard error\n" + \54 " debugging information to standard output and standard error\n" + \
@@ -59,7 +61,7 @@
59def main():61def main():
6062
61 try:63 try:
62 opts, args = getopt.getopt(sys.argv[1:], "hvfs:t:")64 opts, args = getopt.getopt(sys.argv[1:], "hvfsa:t:")
63 except:65 except:
64 Help()66 Help()
65 sys.exit(1)67 sys.exit(1)
@@ -116,45 +118,60 @@
116 # if the user specifies a schema on the command line, use that.118 # if the user specifies a schema on the command line, use that.
117119
118 input_schemafile = None120 input_schemafile = None
121 input_alias = None
119 input_schematron_file = None122 input_schematron_file = None
120 for opt in opts:123 for opt in opts:
121 if opt[0] == "-s":124 if opt[0] == "-s":
122 input_schemafile = opt[1]125 input_schemafile = opt[1]
126 elif opt[0] == "-a":
127 input_alias = opt[1]
123 elif opt[0] == "-t":128 elif opt[0] == "-t":
124 input_schematron_file = opt[1]129 input_schematron_file = opt[1]
125130
126 # if the user has specified a file to work on, use the suffix as a key131 if input_schemafile is not None and input_alias is not None:
127132 debug.deprint("-s and -a are mutually exclusive", 0)
128 if input_filename is not None and input_schemafile is None:133 sys.exit(1)
129 suffix = input_filename.split(".")[-1]134
130 try:135 #schemafile is not given so look up actual path
131 input_schemafile = config.schemata[suffix][1]
132 except:
133 debug.deprint("Could not find schema matching suffix %s." % suffix, 0)
134 debug.deprint("Have you registered it in /etc/diamond/schemata/ or $HOME/.diamond/schemata?", 0)
135 debug.deprint("To register a schema, place a file in one of those directories, and let its name be the suffix of your language.", 0)
136 debug.deprint("The file should have two lines in it:", 0)
137 debug.deprint("A Verbal Description Of The Language Purpose", 0)
138 debug.deprint("/path/to/the/schema/file.rng", 0)
139 sys.exit(1)
140
141 # if there is only one schema, use that
142
143 if input_schemafile is None:136 if input_schemafile is None:
144 if len(config.schemata) == 1:137
138 # if the user has specified a file to work on, use the suffix as a key
139 if input_filename is not None:
140 suffix = input_filename.split(".")[-1]
141 try:
142 schemata = config.schemata[suffix]
143 except:
144 debug.deprint("Could not find schema matching suffix %s." % suffix, 0)
145 debug.deprint("Have you registered it in /etc/diamond/schemata/ or $HOME/.diamond/schemata?", 0)
146 debug.deprint("To register a schema, place a file in one of those directories, and let its name be the suffix of your language.", 0)
147 debug.deprint("The file should consist of:", 0)
148 debug.deprint(" A Verbal Description Of The Language Purpose", 0)
149 debug.deprint(" Lines of the format alias=/path/to/the/schema/file.rng", 0)
150 debug.deprint(" alias= is optional, if not given default= is added automaticly")
151 sys.exit(1)
152
153 # if there is only one schema, use that
154 elif len(config.schemata) == 1:
145 suffix = config.schemata.keys()[0]155 suffix = config.schemata.keys()[0]
146 input_schemafile = config.schemata[suffix][1]156 schemata = config.schemata[suffix]
147157
148 # otherwise ask the user to choose158 # otherwise ask the user to choose
149159 else:
150 if input_schemafile is None:160 choices = [key + ": " + value[0] for key, value in config.schemata.iteritems()]
151 choices = [key + ": " + config.schemata[key][0] for key in config.schemata]161 choice = dialogs.radio_dialog("Choose a schema", "Choose a schema to use:", choices, logofile)
152 choice = dialogs.radio_dialog("Choose a schema", "Choose a schema to use:", choices, logofile)162 suffix = choice.split(":")[0]
153 suffix = choice.split(":")[0]163 schemata = config.schemata[suffix]
154 input_schemafile = config.schemata[suffix][1]164
165 try:
166 #if input_alias is None it will just use the default
167 input_schemafile = schemata[1][input_alias]
168 except KeyError:
169 debug.deprint("""Could not find alias "%s"!""" % (input_alias or "default"), 0)
170 sys.exit(1)
171
172 #endif
155173
156 # ensure that the specified schema actually exists!174 # ensure that the specified schema actually exists!
157
158 try:175 try:
159 if 'http' not in input_schemafile:176 if 'http' not in input_schemafile:
160 os.stat(input_schemafile)177 os.stat(input_schemafile)
161178
=== modified file 'diamond/diamond/config.py'
--- diamond/diamond/config.py 2011-08-01 15:19:52 +0000
+++ diamond/diamond/config.py 2011-08-12 09:02:24 +0000
@@ -29,7 +29,11 @@
2929
30# Here we hard-code a default for flml30# Here we hard-code a default for flml
31# so that users don't have to tweak this to run it.31# so that users don't have to tweak this to run it.
32<<<<<<< TREE
32schemata = {'flml': ('Fluidity markup language', 'http://bazaar.launchpad.net/~fluidity-core/fluidity/4.0-release/download/head:/fluidity_options.rng-20110415014759-hdavpx17hi2vz53z-811/fluidity_options.rng')}33schemata = {'flml': ('Fluidity markup language', 'http://bazaar.launchpad.net/~fluidity-core/fluidity/4.0-release/download/head:/fluidity_options.rng-20110415014759-hdavpx17hi2vz53z-811/fluidity_options.rng')}
34=======
35schemata = {'flml': ('Fluidity markup language', { None: 'http://amcg.ese.ic.ac.uk/svn/fluidity/tags/4.0-release/schemas/fluidity_options.rng'})}
36>>>>>>> MERGE-SOURCE
3337
34for dir in dirs:38for dir in dirs:
35 try:39 try:
@@ -44,30 +48,37 @@
44 except:48 except:
45 debug.deprint("Failure to examine entry " + file + " in folder " + dir + ".")49 debug.deprint("Failure to examine entry " + file + " in folder " + dir + ".")
46 continue50 continue
47 newSchemata = [x.strip() for x in handle]51 lines = [x.strip() for x in handle if x.strip()]
48 if len(newSchemata) < 2:52 if len(lines) < 2:
49 debug.deprint("Warning: Found schema registration file \"" + file + "\", but file is improperly formatted - schema type not registered", 0)53 debug.deprint("Warning: Found schema registration file \"" + file + "\", but file is improperly formatted - schema type not registered", 0)
50 continue54 continue
55
51 # Expand environment variables in the schema path56 # Expand environment variables in the schema path
52 newSchemata[1] = os.path.expandvars(newSchemata[1])57 alias = {}
53 if not os.path.exists(newSchemata[1]) and 'http' not in newSchemata[1]:58 for i in range(1, len(lines)):
54 debug.deprint("Warning: not a valid path: %s" % newSchemata[1], 0)59 line = lines[i]
55 debug.deprint("schema type not registered")60
56 continue61 keyvalue = [x.strip() for x in line.split("=")]
57 schemata[file] = tuple(newSchemata)62 key, value = ("default", keyvalue[0]) if len(keyvalue) == 1 else keyvalue
63
64 value = os.path.expandvars(value)
65 if not os.path.exists(value) and 'http' not in value:
66 debug.deprint("Warning: not a valid path: %s" % value)
67 debug.deprint("schema type not registered")
68 continue
69
70 if key in alias:
71 debug.deprint("""alias "%s" already registered, ignoring""" % key)
72 else:
73 alias[key] = value
74 if key == "default":
75 alias[None] = value
76
77 schemata[file] = (lines[0], alias)
58 debug.dprint("Registered schema type: " + file)78 debug.dprint("Registered schema type: " + file)
59 except OSError:79 except OSError:
60 pass80 pass
6181
62if len(schemata) == 0 and "-s" not in sys.argv:
63 debug.deprint("Error: could not find any registered schemata.", 0)
64 debug.deprint("Have you registered any in one of the directores %s?" % dirs, 0)
65 debug.deprint("To register a schema, place a file in one of those directories, and let its name be the suffix of your language.", 0)
66 debug.deprint("The file should have two lines in it:", 0)
67 debug.deprint(" A Verbal Description Of The Language Purpose", 0)
68 debug.deprint(" /path/to/the/schema/file.rng", 0)
69 sys.exit(1)
70
71if __name__ == "__main__":82if __name__ == "__main__":
72 for key in schemata:83 for key in schemata:
73 debug.dprint("%s: %s" % (key, schemata[key]), 0)84 debug.dprint("%s: %s" % (key, schemata[key]), 0)
7485
=== modified file 'doc/spud_manual.tex'
--- doc/spud_manual.tex 2011-07-29 13:31:43 +0000
+++ doc/spud_manual.tex 2011-08-12 09:02:24 +0000
@@ -1170,10 +1170,11 @@
1170current system. This is specified in the \verb+schemata+ subdirectory of the1170current system. This is specified in the \verb+schemata+ subdirectory of the
1171\verb+/etc/diamond+ and \verb+~/.diamond+ directories. The file names in the1171\verb+/etc/diamond+ and \verb+~/.diamond+ directories. The file names in the
1172\verb+schemata+ directory give the filename extension associated with a1172\verb+schemata+ directory give the filename extension associated with a
1173particular problem description language. The content of the file is two1173particular problem description language. The content of the file is two or
1174lines. The first line is the name of the problem description language while1174more lines. The first line is the name of the problem description language,
1175the second line contains the path of the XML syntax (\verb+.rng+) version of1175the remaining lines are key value pairs (key = value) of alias names and the
1176the corresponding schema.1176path of the XML syntax (\verb+.rng+) version of the corresponding schema.
1177A line with no "key =" part will be interpreted as "default =".
11771178
1178For example, the Fluidity package has a problem description language called1179For example, the Fluidity package has a problem description language called
1179the Fluidity Markup Language which uses the file extension1180the Fluidity Markup Language which uses the file extension

Subscribers

People subscribed via source and target branches