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
1=== modified file 'diamond/bin/diamond'
2--- diamond/bin/diamond 2011-07-05 11:42:44 +0000
3+++ diamond/bin/diamond 2011-08-12 09:02:24 +0000
4@@ -20,6 +20,7 @@
5 import os.path
6 import sys
7 import traceback
8+import string
9
10 import gtk
11 import gtk.gdk
12@@ -47,6 +48,7 @@
13 "-h Display this message\n" + \
14 "-f Forks at startup\n" + \
15 "-s [SCHEMAFILE] Use the supplied schema file *\n" + \
16+ "-a [ALIAS] Use the supplied alias, if not given use \"default\"\n" + \
17 "-t [TRONFILE] Use the supplied schematron file for extended validation\n" + \
18 "-v Verbosity switch - if supplied Diamond prints additional\n" + \
19 " debugging information to standard output and standard error\n" + \
20@@ -59,7 +61,7 @@
21 def main():
22
23 try:
24- opts, args = getopt.getopt(sys.argv[1:], "hvfs:t:")
25+ opts, args = getopt.getopt(sys.argv[1:], "hvfsa:t:")
26 except:
27 Help()
28 sys.exit(1)
29@@ -116,45 +118,60 @@
30 # if the user specifies a schema on the command line, use that.
31
32 input_schemafile = None
33+ input_alias = None
34 input_schematron_file = None
35 for opt in opts:
36 if opt[0] == "-s":
37 input_schemafile = opt[1]
38+ elif opt[0] == "-a":
39+ input_alias = opt[1]
40 elif opt[0] == "-t":
41 input_schematron_file = opt[1]
42
43- # if the user has specified a file to work on, use the suffix as a key
44-
45- if input_filename is not None and input_schemafile is None:
46- suffix = input_filename.split(".")[-1]
47- try:
48- input_schemafile = config.schemata[suffix][1]
49- except:
50- debug.deprint("Could not find schema matching suffix %s." % suffix, 0)
51- debug.deprint("Have you registered it in /etc/diamond/schemata/ or $HOME/.diamond/schemata?", 0)
52- 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)
53- debug.deprint("The file should have two lines in it:", 0)
54- debug.deprint("A Verbal Description Of The Language Purpose", 0)
55- debug.deprint("/path/to/the/schema/file.rng", 0)
56- sys.exit(1)
57-
58- # if there is only one schema, use that
59-
60+ if input_schemafile is not None and input_alias is not None:
61+ debug.deprint("-s and -a are mutually exclusive", 0)
62+ sys.exit(1)
63+
64+ #schemafile is not given so look up actual path
65 if input_schemafile is None:
66- if len(config.schemata) == 1:
67+
68+ # if the user has specified a file to work on, use the suffix as a key
69+ if input_filename is not None:
70+ suffix = input_filename.split(".")[-1]
71+ try:
72+ schemata = config.schemata[suffix]
73+ except:
74+ debug.deprint("Could not find schema matching suffix %s." % suffix, 0)
75+ debug.deprint("Have you registered it in /etc/diamond/schemata/ or $HOME/.diamond/schemata?", 0)
76+ 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)
77+ debug.deprint("The file should consist of:", 0)
78+ debug.deprint(" A Verbal Description Of The Language Purpose", 0)
79+ debug.deprint(" Lines of the format alias=/path/to/the/schema/file.rng", 0)
80+ debug.deprint(" alias= is optional, if not given default= is added automaticly")
81+ sys.exit(1)
82+
83+ # if there is only one schema, use that
84+ elif len(config.schemata) == 1:
85 suffix = config.schemata.keys()[0]
86- input_schemafile = config.schemata[suffix][1]
87-
88- # otherwise ask the user to choose
89-
90- if input_schemafile is None:
91- choices = [key + ": " + config.schemata[key][0] for key in config.schemata]
92- choice = dialogs.radio_dialog("Choose a schema", "Choose a schema to use:", choices, logofile)
93- suffix = choice.split(":")[0]
94- input_schemafile = config.schemata[suffix][1]
95+ schemata = config.schemata[suffix]
96+
97+ # otherwise ask the user to choose
98+ else:
99+ choices = [key + ": " + value[0] for key, value in config.schemata.iteritems()]
100+ choice = dialogs.radio_dialog("Choose a schema", "Choose a schema to use:", choices, logofile)
101+ suffix = choice.split(":")[0]
102+ schemata = config.schemata[suffix]
103+
104+ try:
105+ #if input_alias is None it will just use the default
106+ input_schemafile = schemata[1][input_alias]
107+ except KeyError:
108+ debug.deprint("""Could not find alias "%s"!""" % (input_alias or "default"), 0)
109+ sys.exit(1)
110+
111+ #endif
112
113 # ensure that the specified schema actually exists!
114-
115 try:
116 if 'http' not in input_schemafile:
117 os.stat(input_schemafile)
118
119=== modified file 'diamond/diamond/config.py'
120--- diamond/diamond/config.py 2011-08-01 15:19:52 +0000
121+++ diamond/diamond/config.py 2011-08-12 09:02:24 +0000
122@@ -29,7 +29,11 @@
123
124 # Here we hard-code a default for flml
125 # so that users don't have to tweak this to run it.
126+<<<<<<< TREE
127 schemata = {'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')}
128+=======
129+schemata = {'flml': ('Fluidity markup language', { None: 'http://amcg.ese.ic.ac.uk/svn/fluidity/tags/4.0-release/schemas/fluidity_options.rng'})}
130+>>>>>>> MERGE-SOURCE
131
132 for dir in dirs:
133 try:
134@@ -44,30 +48,37 @@
135 except:
136 debug.deprint("Failure to examine entry " + file + " in folder " + dir + ".")
137 continue
138- newSchemata = [x.strip() for x in handle]
139- if len(newSchemata) < 2:
140+ lines = [x.strip() for x in handle if x.strip()]
141+ if len(lines) < 2:
142 debug.deprint("Warning: Found schema registration file \"" + file + "\", but file is improperly formatted - schema type not registered", 0)
143 continue
144+
145 # Expand environment variables in the schema path
146- newSchemata[1] = os.path.expandvars(newSchemata[1])
147- if not os.path.exists(newSchemata[1]) and 'http' not in newSchemata[1]:
148- debug.deprint("Warning: not a valid path: %s" % newSchemata[1], 0)
149- debug.deprint("schema type not registered")
150- continue
151- schemata[file] = tuple(newSchemata)
152+ alias = {}
153+ for i in range(1, len(lines)):
154+ line = lines[i]
155+
156+ keyvalue = [x.strip() for x in line.split("=")]
157+ key, value = ("default", keyvalue[0]) if len(keyvalue) == 1 else keyvalue
158+
159+ value = os.path.expandvars(value)
160+ if not os.path.exists(value) and 'http' not in value:
161+ debug.deprint("Warning: not a valid path: %s" % value)
162+ debug.deprint("schema type not registered")
163+ continue
164+
165+ if key in alias:
166+ debug.deprint("""alias "%s" already registered, ignoring""" % key)
167+ else:
168+ alias[key] = value
169+ if key == "default":
170+ alias[None] = value
171+
172+ schemata[file] = (lines[0], alias)
173 debug.dprint("Registered schema type: " + file)
174 except OSError:
175 pass
176
177-if len(schemata) == 0 and "-s" not in sys.argv:
178- debug.deprint("Error: could not find any registered schemata.", 0)
179- debug.deprint("Have you registered any in one of the directores %s?" % dirs, 0)
180- 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)
181- debug.deprint("The file should have two lines in it:", 0)
182- debug.deprint(" A Verbal Description Of The Language Purpose", 0)
183- debug.deprint(" /path/to/the/schema/file.rng", 0)
184- sys.exit(1)
185-
186 if __name__ == "__main__":
187 for key in schemata:
188 debug.dprint("%s: %s" % (key, schemata[key]), 0)
189
190=== modified file 'doc/spud_manual.tex'
191--- doc/spud_manual.tex 2011-07-29 13:31:43 +0000
192+++ doc/spud_manual.tex 2011-08-12 09:02:24 +0000
193@@ -1170,10 +1170,11 @@
194 current system. This is specified in the \verb+schemata+ subdirectory of the
195 \verb+/etc/diamond+ and \verb+~/.diamond+ directories. The file names in the
196 \verb+schemata+ directory give the filename extension associated with a
197-particular problem description language. The content of the file is two
198-lines. The first line is the name of the problem description language while
199-the second line contains the path of the XML syntax (\verb+.rng+) version of
200-the corresponding schema.
201+particular problem description language. The content of the file is two or
202+more lines. The first line is the name of the problem description language,
203+the remaining lines are key value pairs (key = value) of alias names and the
204+path of the XML syntax (\verb+.rng+) version of the corresponding schema.
205+A line with no "key =" part will be interpreted as "default =".
206
207 For example, the Fluidity package has a problem description language called
208 the Fluidity Markup Language which uses the file extension

Subscribers

People subscribed via source and target branches