Status: | Merged |
---|---|
Merged at revision: | 481 |
Proposed branch: | lp:~spud/spud/alias |
Merge into: | lp:spud |
Prerequisite: | lp:~spud/spud/slice-view |
Diff against target: |
228 lines (+85/-51) 3 files modified
diamond/bin/diamond (+46/-29) diamond/diamond/config.py (+25/-18) doc/spud_manual.tex (+14/-4) |
To merge this branch: | bzr merge lp:~spud/spud/alias |
Related bugs: | |
Related blueprints: |
Aliases for schema file paths
(Undefined)
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Adam Candy | Approve | ||
Florian Rathgeber | Pending | ||
Review via email:
|
This proposal supersedes a proposal from 2011-07-25.
Commit message
Description of the change
Allows schemata files to list key value pairs of alias name and xml path.

Patrick Farrell (pefarrell) wrote : Posted in a previous version of this proposal | # |

Patrick Farrell (pefarrell) wrote : Posted in a previous version of this proposal | # |
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.

Florian Rathgeber (florian-rathgeber) wrote : Posted in a previous version of this proposal | # |
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.

Florian Rathgeber (florian-rathgeber) wrote : Posted in a previous version of this proposal | # |
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.

Fraser Waters (fraser-waters08) wrote : Posted in a previous version of this proposal | # |
> 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.

Adam Candy (asc) wrote : Posted in a previous version of this proposal | # |
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/
3. Currently the invalid path warning, e.g.
Warning: not a valid path: /home/asc/
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.

Stephan Kramer (s-kramer) wrote : Posted in a previous version of this proposal | # |
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.

Fraser Waters (fraser-waters08) wrote : Posted in a previous version of this proposal | # |
@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?

Florian Rathgeber (florian-rathgeber) wrote : Posted in a previous version of this proposal | # |
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.

Fraser Waters (fraser-waters08) wrote : Posted in a previous version of this proposal | # |
> 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.

Patrick Farrell (pefarrell) wrote : Posted in a previous version of this proposal | # |
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.

Patrick Farrell (pefarrell) wrote : | # |
Hi Fraser,
Can you please document your changes in the manual? People need to be able to discover this feature. At a minimum, you should add some text to sections 4.2.1 and 4.2.2.
- 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

Adam Candy (asc) wrote : | # |
This looks good to me - I can put away the wrapper I've been using!
The only comment I have relates to my point 3 above, using an alias with an invalid path gives the error:
Could not find alias "aliasname"!
when actually the alias does exist, it's just that the path is invalid. Something like the following might be more suitable:
Path 'path' for alias 'aliasname' does not exist.

Fraser Waters (fraser-waters08) wrote : | # |
Thats fixed, it was the case that it couldn't find the alias, but only because the alias loader was checking paths were valid (superfluous as it's done later)
Preview Diff
1 | === modified file 'diamond/bin/diamond' |
2 | --- diamond/bin/diamond 2011-08-24 16:51:39 +0000 |
3 | +++ diamond/bin/diamond 2011-08-25 14:38:35 +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 | @@ -49,6 +50,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 | "-d [FILE] Diff against the supplied file. (FILE must be specified)\n" + \ |
19 | "-v Verbosity switch - if supplied Diamond prints additional\n" + \ |
20 | @@ -62,7 +64,7 @@ |
21 | def main(): |
22 | |
23 | try: |
24 | - opts, args = getopt.gnu_getopt(sys.argv[1:], "hvfs:t:d:") |
25 | + opts, args = getopt.gnu_getopt(sys.argv[1:], "hvfs:a:t:d:") |
26 | except: |
27 | Help() |
28 | sys.exit(1) |
29 | @@ -119,45 +121,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-25 14:38:35 +0000 |
122 | @@ -29,7 +29,7 @@ |
123 | |
124 | # Here we hard-code a default for flml |
125 | # so that users don't have to tweak this to run it. |
126 | -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')} |
127 | +schemata = {'flml': ('Fluidity markup language', { None: 'http://bazaar.launchpad.net/~fluidity-core/fluidity/4.0-release/download/head:/fluidity_options.rng-20110415014759-hdavpx17hi2vz53z-811/fluidity_options.rng'})} |
128 | |
129 | for dir in dirs: |
130 | try: |
131 | @@ -44,30 +44,37 @@ |
132 | except: |
133 | debug.deprint("Failure to examine entry " + file + " in folder " + dir + ".") |
134 | continue |
135 | - newSchemata = [x.strip() for x in handle] |
136 | - if len(newSchemata) < 2: |
137 | + lines = [x.strip() for x in handle if x.strip()] |
138 | + if len(lines) < 2: |
139 | debug.deprint("Warning: Found schema registration file \"" + file + "\", but file is improperly formatted - schema type not registered", 0) |
140 | continue |
141 | + |
142 | # Expand environment variables in the schema path |
143 | - newSchemata[1] = os.path.expandvars(newSchemata[1]) |
144 | - if not os.path.exists(newSchemata[1]) and 'http' not in newSchemata[1]: |
145 | - debug.deprint("Warning: not a valid path: %s" % newSchemata[1], 0) |
146 | - debug.deprint("schema type not registered") |
147 | - continue |
148 | - schemata[file] = tuple(newSchemata) |
149 | + alias = {} |
150 | + for i in range(1, len(lines)): |
151 | + line = lines[i] |
152 | + |
153 | + keyvalue = [x.strip() for x in line.split("=")] |
154 | + key, value = ("default", keyvalue[0]) if len(keyvalue) == 1 else keyvalue |
155 | + |
156 | + value = os.path.expandvars(value) |
157 | + if not os.path.exists(value) and 'http' not in value: |
158 | + debug.deprint("Warning: not a valid path: %s" % value) |
159 | + debug.deprint("schema type not registered") |
160 | + continue |
161 | + |
162 | + if key in alias: |
163 | + debug.deprint("""alias "%s" already registered, ignoring""" % key) |
164 | + else: |
165 | + alias[key] = value |
166 | + if key == "default": |
167 | + alias[None] = value |
168 | + |
169 | + schemata[file] = (lines[0], alias) |
170 | debug.dprint("Registered schema type: " + file) |
171 | except OSError: |
172 | pass |
173 | |
174 | -if len(schemata) == 0 and "-s" not in sys.argv: |
175 | - debug.deprint("Error: could not find any registered schemata.", 0) |
176 | - debug.deprint("Have you registered any in one of the directores %s?" % dirs, 0) |
177 | - 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) |
178 | - debug.deprint("The file should have two lines in it:", 0) |
179 | - debug.deprint(" A Verbal Description Of The Language Purpose", 0) |
180 | - debug.deprint(" /path/to/the/schema/file.rng", 0) |
181 | - sys.exit(1) |
182 | - |
183 | if __name__ == "__main__": |
184 | for key in schemata: |
185 | debug.dprint("%s: %s" % (key, schemata[key]), 0) |
186 | |
187 | === modified file 'doc/spud_manual.tex' |
188 | --- doc/spud_manual.tex 2011-08-22 16:29:08 +0000 |
189 | +++ doc/spud_manual.tex 2011-08-25 14:38:35 +0000 |
190 | @@ -1388,10 +1388,11 @@ |
191 | current system. This is specified in the \verb+schemata+ subdirectory of the |
192 | \verb+/etc/diamond+ and \verb+~/.diamond+ directories. The file names in the |
193 | \verb+schemata+ directory give the filename extension associated with a |
194 | -particular problem description language. The content of the file is two |
195 | -lines. The first line is the name of the problem description language while |
196 | -the second line contains the path of the XML syntax (\verb+.rng+) version of |
197 | -the corresponding schema. |
198 | +particular problem description language. The content of the file is two or |
199 | +more lines. The first line is the name of the problem description language, |
200 | +the remaining lines are key value pairs (key = value) of alias names and the |
201 | +path of the XML syntax (\verb+.rng+) version of the corresponding schema. |
202 | +A line with no "key =" part will be interpreted as "default =". |
203 | |
204 | For example, the Fluidity package has a problem description language called |
205 | the Fluidity Markup Language which uses the file extension |
206 | @@ -1412,6 +1413,12 @@ |
207 | \end{verbatim} |
208 | Now Diamond will pick up the version of the schema in \verb+jrluser+'s svn |
209 | tree rather than the version the sysadmin installed. |
210 | +Alternativly they could just add an alias to the existing \verb+//home/jrluser/.diamond/schemata/flml+: |
211 | +\begin{verbatim} |
212 | +Fluidity Markup Language |
213 | +/usr/share/fluidity/fluidity_options.rng |
214 | +jrl = /home/jrluser/fluidity/tools/fluidity_options.rng |
215 | +\end{verbatim} |
216 | |
217 | It is also possible to specify a URL over HTTP for the location of the |
218 | schema. For example: |
219 | @@ -1446,6 +1453,9 @@ |
220 | Diamond will inspect the registered schemas for the suffix specified |
221 | and use that schema. |
222 | |
223 | +Except for when \verb+-s+ is given \verb+-a ALIAS+ can be supplied to select an alias |
224 | +other than the default. If \verb+-a+ is not given \verb+-a default+ is used. |
225 | + |
226 | \subsection{Dynamic validation} |
227 | Diamond uses the schema to guide the editing of valid documents that the schema |
228 | permits. When a blank document is opened, information necessary to make a valid |
Can you give an example of the new format in the manual, too?