Menu
▾
▴
clops-users — mailing lists for the users of CLOPS
This list is closed, nobody may subscribe to it.
| 2009 |
Jan
|
Feb
|
Mar
(3) |
Apr
(37) |
May
(4) |
Jun
(1) |
Jul
(18) |
Aug
(12) |
Sep
(4) |
Oct
|
Nov
|
Dec
|
|---|
|
From: Radu G. <rad...@gm...> - 2009-09-30 11:06:28
|
I have implemented a toy version of the ambiguity check algorithm (in ocaml). While doing so I realized that our algorithm is the *same* as that of Even if we plug in the correct stopping condition C. I did not implement the black/white nodes thing (so ambiguities *within* one option aren't ignored). For automata with at most 2000 nodes and at most 2000 edges the worst case is solved in <0.6s. That worst case looks like [0]. The problem is here: https://www.spoj.pl/problems/AMBIG/ I'm not very happy with the tests: I think there must be some more interesting cases than the ones I came up with. While writing the tests I noticed that my worry about multiplying lengths of cycles was misguided: It's never the case that more than two cycles interact! The first solution submitted by someone else to that problem is in C++ and is about 8 times faster than my ocaml, so we can probably safely assume that <0.6s can be made <0.1s. I'd say it's fast enough for CLOPS. What do you think? [0] http://tinyurl.com/ydctoju |
|
From: Mikoláš J. <mik...@gm...> - 2009-09-07 07:40:37
|
I implemented a class for generating edit changes. It is implemented
as an Iterator<String> and it generates changes in order of distance
from the beginning of the considered string.
Why Iterator?
we might have a different mechanism for generating changes, the
core algo can treat any of those mechanisms as Iterators
(minor memory gain as the edits are not kept in memory)
Why editing from the beginning?
we should consider fixing more than just one word (i.e. up to
separator). then we could be editing things farther down from the
place where the parsing went wrong, but fixing the place close to the
error point is preferable
The target run-Variants does a dry run of the generator. The order for
"abc" and modification alphabet 'x', 'y' is this:
edits at 0 [xabc, xbc, yabc, bc, ybc, bac]
edits at 1 [axc, axbc, acb, ac, ayc, aybc]
edits at 2 [abcx, abcy, abx, aby, abyc, ab, abxc] // contains
insertion of the last char as well
--
Mikoláš
|
|
From: Mikoláš J. <mik...@gm...> - 2009-09-05 14:16:13
|
great, thanks! I moved the test file into the test hierarchy and added a junit target, which gets run with the other tests (ant test) -- mikolas On Sat, Sep 5, 2009 at 2:00 AM, Viliam Holub<vil...@uc...> wrote: > > First version of tab completion committed. > > src/java/ie/ucd/clops/runtime/tabcomplete/TabComplete.java > src/java/ie/ucd/clops/runtime/tabcomplete/TabCompleteTest.java > > The key function is: > List<String> complete( final String regexp, final String prefix, int limit); > > There are jUnit tests in TabCompleteTest.java, but I wasn't sure how to add > that to build properly, co it's not called now. Help wanted :) > > Viliam > > > ------------------------------------------------------------------------------ > Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day > trial. Simplify your report design, integration and deployment - and focus on > what you do best, core application coding. Discover what's new with > Crystal Reports now. http://p.sf.net/sfu/bobj-july > _______________________________________________ > Clops-users mailing list > Clo...@li... > https://lists.sourceforge.net/lists/listinfo/clops-users > -- Mikoláš Janota M. Sc. School of Computer Science and Informatics, University College Dublin, Belfield, Dublin 4, Ireland |
|
From: Viliam H. <vil...@uc...> - 2009-09-05 01:00:52
|
First version of tab completion committed. src/java/ie/ucd/clops/runtime/tabcomplete/TabComplete.java src/java/ie/ucd/clops/runtime/tabcomplete/TabCompleteTest.java The key function is: List<String> complete( final String regexp, final String prefix, int limit); There are jUnit tests in TabCompleteTest.java, but I wasn't sure how to add that to build properly, co it's not called now. Help wanted :) Viliam |
|
From: Radu G. <rad...@gm...> - 2009-08-12 18:00:34
|
On Wed, Aug 12, 2009 at 3:50 PM, Mikoláš Janota<mik...@gm...> wrote:
> it seems silly to write
> s:{"-s"}:{string-regexp} :[between="" regexp="a*" blankparamallowed="false"]
Especially when the default for blankParamAllowed is false.
> instead of
> s:{"-s"}:{string-regexp} :[between="" regexp="a+" ]
A better comparison would be with
s:{"-s"}:{string-regexp} :[between="" regexp="a+" blanparamallowed]
so that we simulate that blankParamAllowed check was removed from parse().
What do we see? An error message that doesn't mention the alias that
was typed, as I said. (There is, however, a ^ pointing to the right
place thanks to the latest additions by Fintan.)
Not to mention that string-regexp is quite different from string in
that it has validation and the whole point of using string as an
example in my previous emails was as an option without validation.
This discussion is getting silly. I'll stop now.
|
|
From: Mikoláš J. <mik...@gm...> - 2009-08-12 14:50:28
|
On Wed, Aug 12, 2009 at 2:50 PM, Radu Grigore<rad...@gm...> wrote:
> On Wed, Aug 12, 2009 at 1:58 PM, Mikoláš Janota<mik...@gm...> wrote:
>> From what you are saying I can't figure out the difference between a
>> blank arg and malformed arg.
>
> It's nice to have a different error message and treat strings and
> integers uniformly.
>
if the example with integers bothers you, consider
s:{"-s"}:{string-regexp} :[between="" regexp="a+" ]
why should -sxxx be considered differently from -s"" ? both "" and
"xxx" are wrong params
(if you want different message just print "" differently)
it seems silly to write
s:{"-s"}:{string-regexp} :[between="" regexp="a*" blankparamallowed="false"]
instead of
s:{"-s"}:{string-regexp} :[between="" regexp="a+" ]
we could probably add a check whether an "" matches the arguments
shape once "" returns as the arg
--m.
m.
>
>> Nevertheless, we seem to have a problem as "tail -nBAZOOM" gives me
>> Illegal option: -nBAZOOM
>> This means we don't handle well malformed args.
>
> It looks more like tail.clo is wrong.
>
--
Mikoláš Janota M. Sc.
School of Computer Science and Informatics,
University College Dublin,
Belfield,
Dublin 4,
Ireland
|
|
From: Mikoláš J. <mik...@gm...> - 2009-08-12 14:14:58
|
On Wed, Aug 12, 2009 at 12:56 PM, Radu Grigore<rad...@gm...> wrote: > On Tue, Aug 11, 2009 at 10:06 PM, Mikoláš > Janota<mik...@gm...> wrote: >> How's this different from >> head -nBAZOOM, ie the arg is there but it's not a digit? > > You get a different error message (unless there's a default value set). > I don't understand. I was saying that both -nBAZOOM and -n"" are bad arguments. We might decide to print something else than "". We could say something like "empty argument is not valid for -n" when we get "" instead of "BAZOOM" In other words, !blankparamallowed can be derived from the fact that the option matched, didn't parse, and the string corresponding to the argument is empty. --m >> As Fint noted, in infinite lookahead this is more trouble. > > The check should be moved out of process(), yes. In general, process() > needs to be deprecated if we want to be able to keep the parsers in > sync. > -- Mikoláš Janota M. Sc. School of Computer Science and Informatics, University College Dublin, Belfield, Dublin 4, Ireland |
|
From: Radu G. <rad...@gm...> - 2009-08-12 13:50:56
|
On Wed, Aug 12, 2009 at 1:58 PM, Mikoláš Janota<mik...@gm...> wrote: > From what you are saying I can't figure out the difference between a > blank arg and malformed arg. It's nice to have a different error message and treat strings and integers uniformly. > Nevertheless, we seem to have a problem as "tail -nBAZOOM" gives me > Illegal option: -nBAZOOM > This means we don't handle well malformed args. It looks more like tail.clo is wrong. |
|
From: Mikoláš J. <mik...@gm...> - 2009-08-12 12:59:04
|
>From what you are saying I can't figure out the difference between a
blank arg and malformed arg. And I believe that differentiating them
is a bad design.
Nevertheless, we seem to have a problem as "tail -nBAZOOM" gives me
Illegal option: -nBAZOOM
This means we don't handle well malformed args. The blankparamallowed
is just a heck that makes it work well if the malformed arg is a
blank.
m.
On Wed, Aug 12, 2009 at 1:36 PM, Radu Grigore<rad...@gm...> wrote:
> On Wed, Aug 12, 2009 at 1:21 PM, Mikoláš Janota<mik...@gm...> wrote:
>> In other words, !blankparamallowed can be derived from the fact that
>> the option matched, didn't parse, and the string corresponding to the
>> argument is empty.
>
> That's how it used to work: IntegerOption.convertStringToValue does
> exactly that. In that case string options need to be handled with
> validity constraints, while integer options do not need a validity
> constraint. A small issue with this is that convertStringToValue
> doesn't know the text that matched on the prefix, so it can't say
> (now) "option -foo requires an argument", where "-foo" is the
> particular alias that was typed.
>
--
Mikoláš Janota M. Sc.
School of Computer Science and Informatics,
University College Dublin,
Belfield,
Dublin 4,
Ireland
|
|
From: Radu G. <rad...@gm...> - 2009-08-12 12:36:39
|
On Wed, Aug 12, 2009 at 1:21 PM, Mikoláš Janota<mik...@gm...> wrote: > In other words, !blankparamallowed can be derived from the fact that > the option matched, didn't parse, and the string corresponding to the > argument is empty. That's how it used to work: IntegerOption.convertStringToValue does exactly that. In that case string options need to be handled with validity constraints, while integer options do not need a validity constraint. A small issue with this is that convertStringToValue doesn't know the text that matched on the prefix, so it can't say (now) "option -foo requires an argument", where "-foo" is the particular alias that was typed. |
|
From: Radu G. <rad...@gm...> - 2009-08-12 11:56:56
|
On Tue, Aug 11, 2009 at 10:06 PM, Mikoláš Janota<mik...@gm...> wrote: > How's this different from > head -nBAZOOM, ie the arg is there but it's not a digit? You get a different error message (unless there's a default value set). > As Fint noted, in infinite lookahead this is more trouble. The check should be moved out of process(), yes. In general, process() needs to be deprecated if we want to be able to keep the parsers in sync. |
|
From: Mikoláš J. <mik...@gm...> - 2009-08-11 21:06:53
|
On Tue, Aug 11, 2009 at 4:09 PM, Radu Grigore<rad...@gm...> wrote: > On Sun, Aug 9, 2009 at 12:20 AM, Mikoláš Janota<mik...@gm...> wrote: >> I still don't understand what blankParamAllowed is good for. > > The error message "option X needs an argument" is better than "i don't > know what this option is". > How's this different from head -nBAZOOM, ie the arg is there but it's not a digit? We can of course print an empty string in some different fashion than BAZOOM but I don't see the need for the blank param allowed property. I thought that that getMatchingOption says that the option matched and parse that it didn't work out for exactly this reason. As Fint noted, in infinite lookahead this is more trouble. -- Mikoláš |
|
From: Fintan F. <fi...@gm...> - 2009-08-11 20:21:46
|
Yes, this is it exactly. By having a boolean property to control this, we allow the option to match regardless of whether the argument is blank (empty string) in the option's regexp. Then, when we process we can give a useful error message if we do not allow blank arguments. If we do allow blank arguments, we just proceed. This raises an issue though. In the infinite lookahead parser we don't use process, so this case (and the interaction with using the newly introduced "defaultVal") must be handled separately. On Tue, Aug 11, 2009 at 4:09 PM, Radu Grigore <rad...@gm...> wrote: > On Sun, Aug 9, 2009 at 12:20 AM, Mikoláš Janota<mik...@gm...> > wrote: > > I still don't understand what blankParamAllowed is good for. > > The error message "option X needs an argument" is better than "i don't > know what this option is". > > > ------------------------------------------------------------------------------ > Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day > trial. Simplify your report design, integration and deployment - and focus > on > what you do best, core application coding. Discover what's new with > Crystal Reports now. http://p.sf.net/sfu/bobj-july > _______________________________________________ > Clops-users mailing list > Clo...@li... > https://lists.sourceforge.net/lists/listinfo/clops-users > |
|
From: Radu G. <rad...@gm...> - 2009-08-11 15:09:29
|
On Sun, Aug 9, 2009 at 12:20 AM, Mikoláš Janota<mik...@gm...> wrote: > I still don't understand what blankParamAllowed is good for. The error message "option X needs an argument" is better than "i don't know what this option is". |
|
From: Mikoláš J. <mik...@gm...> - 2009-08-08 23:20:39
|
I still don't understand what blankParamAllowed is good for. At the moment is just checked in process but argumentshape can take care of that. One possibility would be for the property to set between and argumentshape somehow. But I'm not quite sure how plus I don't like too much the overlaps between properties. I'd suggest to get rid of the property all together. -- Mikoláš |
|
From: Fintan F. <fi...@gm...> - 2009-08-01 12:44:32
|
I have just turned off the ability to create anonymous postings on tracker items (bug reports & feature requests). The reason I made the change is that once again I managed to make postings as an anonymous user before realising that I wasn't signed in. This is the setting recommended by sourceforge anyway, due to potential spam issues. The only drawback is that it significantly increases the time it takes a non-registered user to submit a bug/feature request. If anybody feels strongly that we should continue to permit anonymous bug reports/requests/comments please speak up now. -F |
|
From: Radu G. <rad...@gm...> - 2009-07-31 18:16:16
|
Please avoid private discussions: http://producingoss.com/en/setting-tone.html#avoid-private-discussions On Fri, Jul 31, 2009 at 6:02 PM, Mikoláš Janota<mik...@gm...> wrote: > Radu has rewritten the main pg of clops. I'd say I've done about 5% of the rewriting, and it's not only on the main page. > Personally I think the color scheme could still be improved, I found > this page with color schemes; it might be of help > http://www.colorschemer.com/schemes/ I'm using http://colorschemedesigner.com/ and test how it looks on all pages, rather than on four small squares. I tried about ten schemes until now. You are welcome to organize a vote. You can change colors-1, colors-2, colors-3 in clops.css, take a snapshot and use something like Google Moderator for voting. Please note that you should prefer three-color schemes, because most web-pages have three types of elements (for example, reference.html has section, option, property). A different number of colors will require changing the content to keep colors balanced. > I'm also thinking there should be more explanations for the foo.clo, > for instance as comments in the file. That's the least of problems with that example. A bit more important is that it doesn't work at all. For example, there is no printUsage() method in parser. > Maybe Dragan or Vieri who have no experience with CLOPS > could comment on this? Since the page will likely change a lot, I'm not sure if commenting at this stage makes sense. Newcomers are a scarce resource and we shouldn't waste them on stuff that we *know* is not good. You might want to take a look at reference.html since you wrote it originally. At the beginning there is a proposal for how to reformat/present information. |
|
From: Fintan F. <fi...@gm...> - 2009-07-29 14:11:09
|
I put it in as a temporary fix. The parser and lexer generation was not happening automatically for me as a result of calling ant compile. I've checked the dependencies and they seem to be ok. I fixed a couple of small things in the antlr.xml ant build file, and everything seems to trigger correctly now, so I've taken them back out of the repo. On Wed, Jul 29, 2009 at 12:41 PM, Radu Grigore <rad...@gm...>wrote: > I'm generally against holding generated stuff in the repository. It > tends to cause unnecessary conflicts. The guy who coordinates SVN > development seems to agree: "In general, version control > should only contain things that are "source", i.e. not derivable from > other things. " [0]. Now, that doesn't mean you should never do it. As > Fintan explained me, the CLOPS interface, which is generated, is in > the repo because it makes bootstrapping a non-issue for newcomers. > Otherwise it would be a headache. > > That said, why are CLOParser.java and CLOLexer.java in repo now? > > [0] http://tinyurl.com/r8kytx > > > ------------------------------------------------------------------------------ > Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day > trial. Simplify your report design, integration and deployment - and focus > on > what you do best, core application coding. Discover what's new with > Crystal Reports now. http://p.sf.net/sfu/bobj-july > _______________________________________________ > Clops-users mailing list > Clo...@li... > https://lists.sourceforge.net/lists/listinfo/clops-users > |
|
From: Radu G. <rad...@gm...> - 2009-07-29 12:46:34
|
I'm generally against holding generated stuff in the repository. It tends to cause unnecessary conflicts. The guy who coordinates SVN development seems to agree: "In general, version control should only contain things that are "source", i.e. not derivable from other things. " [0]. Now, that doesn't mean you should never do it. As Fintan explained me, the CLOPS interface, which is generated, is in the repo because it makes bootstrapping a non-issue for newcomers. Otherwise it would be a headache. That said, why are CLOParser.java and CLOLexer.java in repo now? [0] http://tinyurl.com/r8kytx |
|
From: Radu G. <rad...@gm...> - 2009-07-27 12:24:18
|
On Mon, Jul 27, 2009 at 1:18 PM, Radu Grigore<rad...@gm...> wrote: > On Mon, Jul 27, 2009 at 11:43 AM, Fintan Fairmichael<fi...@gm...> wrote: >> We should obviously still expose the list though, so my suggestion would be >> that the wrapper extends List<CLError>. > > I would prefer if we simply have a method like [...] Actually you are right, we can implement Iterable<CLError> too. I don't know why I was thinking that List<CLError> is a class. |
|
From: Radu G. <rad...@gm...> - 2009-07-27 12:19:33
|
On Mon, Jul 27, 2009 at 11:43 AM, Fintan Fairmichael<fi...@gm...> wrote:
> We should obviously still expose the list though, so my suggestion would be
> that the wrapper extends List<CLError>.
I would prefer if we simply have a method like
public class ParseResult {
public void List<CLError> listErrors() { ...}
}
This way we can change (if needed in future versions) the internal
representation easily to anything that can still be *converted* to a
list.
|
|
From: Radu G. <rad...@gm...> - 2009-07-27 12:16:01
|
On Sun, Jul 26, 2009 at 10:44 PM, Mikoláš Janota<mik...@gm...> wrote: > - for some users it might be enough to just say the usage and drop the > error messages (e.g. 'cp foo' just does that). rg@rg-ucd:cfind$ cp foo cp: missing destination file operand after `foo' Try `cp --help' for more information. > - it seems to me there's some additional burden on the user to know > that the parsing was ok iff the list of errors is empty To me it doesn't. Most people would assume that OK = there are no errors Any other behavior would be surprising. > - where does the report method come from? providing and external > printing might could be useful if we wanted to provide several > standard ways of printing errors but the user has to find those > somewhere (some additional doc reading required) >From CLError as I said. And the doc required is not "additional" at all: The difference is only you put the method and, as you saw, from the point of view of calling the method it looks the same. > all in all, do we go for the wrapper then? Yes. I believe I gave a good reason for that: > On Sun, Jul 26, 2009 at 9:53 PM, Radu Grigore<rad...@gm...> wrote: >> So, why would we use a wrapping class? The best reason I see is that >> it gives us a layer of abstraction that we could exploit later on if >> we have to make changes. In other words, it gives us more freedom to >> change the implementation while keeping the interface unchanged. For >> example, if for some reason we notice that errors are better organized >> in a tree (say, A is a child of B if A seems to be causing B) then we >> can just _add_ functions to the interface that expose the tree >> structure, keep reportErrors(), and *nothing* would need to change in >> the user code. -- A: Because it fouls the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing on usenet and in e-mail? |
|
From: Fintan F. <fi...@gm...> - 2009-07-27 10:44:00
|
For implementing functionality like printing the errors to a PrintStream I'd
suggest that we do create a wrapper and put it there.
We should obviously still expose the list though, so my suggestion would be
that the wrapper extends List<CLError>.
For the isOK() method, perhaps successfulParse() would be a better name?
On Sun, Jul 26, 2009 at 10:44 PM, Mikoláš Janota
<mik...@gm...>wrote:
> couple of remarks:
>
> - for some users it might be enough to just say the usage and drop the
> error messages (e.g. 'cp foo' just does that).
>
> - it seems to me there's some additional burden on the user to know
> that the parsing was ok iff the list of errors is empty
>
> - where does the report method come from? providing and external
> printing might could be useful if we wanted to provide several
> standard ways of printing errors but the user has to find those
> somewhere (some additional doc reading required)
>
> all in all, do we go for the wrapper then?
>
> m.
>
>
> On Sun, Jul 26, 2009 at 9:53 PM, Radu Grigore<rad...@gm...>
> wrote:
> > On Sun, Jul 26, 2009 at 3:54 PM, Fintan Fairmichael<fi...@gm...>
> wrote:
> >> We discussed this on Friday and had already agreed such.
> >
> > That said, both reasons given by Mikolas are invalid.
> >
> >> On Sun, Jul 26, 2009 at 1:23 PM, Mikoláš Janota <
> mik...@gm...>
> >> wrote:
> >>> a) To get the previous code to compile I added isEmpty() which looks
> >>> unintuitive. isOK would look nicer.
> >
> > If the old version was
> > if (!p.parse(args)) { printUsage(); return; }
> > you can't simply transform it to
> > if (!p.parse(args).isEmpty()) { ... }
> > because that would loose the error messages that where previously
> > reported as exceptions. If you want to not loose functionality you
> > must say:
> > List<CLError> errors = p.parse(args);
> > if (!errors.isEmpty()) {
> > report(errors, System.err); // well... if report() is added to CLError
> > printUsage();
> > return;
> > }
> > In fact, this is slightly better than the original because it prints
> > the usage information whenever the user had an error on the command
> > line, not only for those errors that just happened not to throw. In
> > this form, it seems quite natural to test whether there are any errors
> > by saying "!errors.isEmpty()" and isOK() would look strange.
> >
> > In short, the examples are incorrect now. But they should be fixed
> > after these API changes are stabilized anyway.
> >
> >>> b) The standard toString of List is probably not desirable for
> >>> presenting to the user the aforementioned class could have some
> >>> printMe method, presenting the errors nicely.
> >
> > With a list you say
> > report(errors, System.err);
> > and with a wrapping class you say
> > parsingResult.reportErrors(System.err);
> > which is not much different.
> >
> > The toString() method is for debugging (although there are some *very*
> > rare situations when I'd use it in a different way). I don't think
> > anyone ever considered it for reporting the parsing errors.
> >
> > So, why would we use a wrapping class? The best reason I see is that
> > it gives us a layer of abstraction that we could exploit later on if
> > we have to make changes. In other words, it gives us more freedom to
> > change the implementation while keeping the interface unchanged. For
> > example, if for some reason we notice that errors are better organized
> > in a tree (say, A is a child of B if A seems to be causing B) then we
> > can just _add_ functions to the interface that expose the tree
> > structure, keep reportErrors(), and *nothing* would need to change in
> > the user code.
> >
>
>
>
> --
> Mikoláš Janota M. Sc.
> School of Computer Science and Informatics,
> University College Dublin,
> Belfield,
> Dublin 4,
> Ireland
>
|
|
From: Mikoláš J. <mik...@gm...> - 2009-07-26 21:44:18
|
couple of remarks:
- for some users it might be enough to just say the usage and drop the
error messages (e.g. 'cp foo' just does that).
- it seems to me there's some additional burden on the user to know
that the parsing was ok iff the list of errors is empty
- where does the report method come from? providing and external
printing might could be useful if we wanted to provide several
standard ways of printing errors but the user has to find those
somewhere (some additional doc reading required)
all in all, do we go for the wrapper then?
m.
On Sun, Jul 26, 2009 at 9:53 PM, Radu Grigore<rad...@gm...> wrote:
> On Sun, Jul 26, 2009 at 3:54 PM, Fintan Fairmichael<fi...@gm...> wrote:
>> We discussed this on Friday and had already agreed such.
>
> That said, both reasons given by Mikolas are invalid.
>
>> On Sun, Jul 26, 2009 at 1:23 PM, Mikoláš Janota <mik...@gm...>
>> wrote:
>>> a) To get the previous code to compile I added isEmpty() which looks
>>> unintuitive. isOK would look nicer.
>
> If the old version was
> if (!p.parse(args)) { printUsage(); return; }
> you can't simply transform it to
> if (!p.parse(args).isEmpty()) { ... }
> because that would loose the error messages that where previously
> reported as exceptions. If you want to not loose functionality you
> must say:
> List<CLError> errors = p.parse(args);
> if (!errors.isEmpty()) {
> report(errors, System.err); // well... if report() is added to CLError
> printUsage();
> return;
> }
> In fact, this is slightly better than the original because it prints
> the usage information whenever the user had an error on the command
> line, not only for those errors that just happened not to throw. In
> this form, it seems quite natural to test whether there are any errors
> by saying "!errors.isEmpty()" and isOK() would look strange.
>
> In short, the examples are incorrect now. But they should be fixed
> after these API changes are stabilized anyway.
>
>>> b) The standard toString of List is probably not desirable for
>>> presenting to the user the aforementioned class could have some
>>> printMe method, presenting the errors nicely.
>
> With a list you say
> report(errors, System.err);
> and with a wrapping class you say
> parsingResult.reportErrors(System.err);
> which is not much different.
>
> The toString() method is for debugging (although there are some *very*
> rare situations when I'd use it in a different way). I don't think
> anyone ever considered it for reporting the parsing errors.
>
> So, why would we use a wrapping class? The best reason I see is that
> it gives us a layer of abstraction that we could exploit later on if
> we have to make changes. In other words, it gives us more freedom to
> change the implementation while keeping the interface unchanged. For
> example, if for some reason we notice that errors are better organized
> in a tree (say, A is a child of B if A seems to be causing B) then we
> can just _add_ functions to the interface that expose the tree
> structure, keep reportErrors(), and *nothing* would need to change in
> the user code.
>
--
Mikoláš Janota M. Sc.
School of Computer Science and Informatics,
University College Dublin,
Belfield,
Dublin 4,
Ireland
|
|
From: Radu G. <rad...@gm...> - 2009-07-26 20:53:15
|
On Sun, Jul 26, 2009 at 3:54 PM, Fintan Fairmichael<fi...@gm...> wrote:
> We discussed this on Friday and had already agreed such.
That said, both reasons given by Mikolas are invalid.
> On Sun, Jul 26, 2009 at 1:23 PM, Mikoláš Janota <mik...@gm...>
> wrote:
>> a) To get the previous code to compile I added isEmpty() which looks
>> unintuitive. isOK would look nicer.
If the old version was
if (!p.parse(args)) { printUsage(); return; }
you can't simply transform it to
if (!p.parse(args).isEmpty()) { ... }
because that would loose the error messages that where previously
reported as exceptions. If you want to not loose functionality you
must say:
List<CLError> errors = p.parse(args);
if (!errors.isEmpty()) {
report(errors, System.err); // well... if report() is added to CLError
printUsage();
return;
}
In fact, this is slightly better than the original because it prints
the usage information whenever the user had an error on the command
line, not only for those errors that just happened not to throw. In
this form, it seems quite natural to test whether there are any errors
by saying "!errors.isEmpty()" and isOK() would look strange.
In short, the examples are incorrect now. But they should be fixed
after these API changes are stabilized anyway.
>> b) The standard toString of List is probably not desirable for
>> presenting to the user the aforementioned class could have some
>> printMe method, presenting the errors nicely.
With a list you say
report(errors, System.err);
and with a wrapping class you say
parsingResult.reportErrors(System.err);
which is not much different.
The toString() method is for debugging (although there are some *very*
rare situations when I'd use it in a different way). I don't think
anyone ever considered it for reporting the parsing errors.
So, why would we use a wrapping class? The best reason I see is that
it gives us a layer of abstraction that we could exploit later on if
we have to make changes. In other words, it gives us more freedom to
change the implementation while keeping the interface unchanged. For
example, if for some reason we notice that errors are better organized
in a tree (say, A is a child of B if A seems to be causing B) then we
can just _add_ functions to the interface that expose the tree
structure, keep reportErrors(), and *nothing* would need to change in
the user code.
|