On 26 August 2016 at 10:31, Colin Fleming <colin.mailingl...@gmail.com> wrote:
> I agree that tidied up the error messages are much more understandable. > Replacing things like "path" with a description of what it means goes a > long way. My main issue with the original error which persists in your > version is that the failing predicate really doesn't help much identifying > the problem. However Leon's investigations hopefully will help to make that > better by more precisely identifying the failing predicate. > Yes the failing predicate isn't always very enlightening, and something like Leon's suggestion seems to match my intuitions about being more accurate, more frequently - though I'll leave judgement on that to people who know this stuff much better than I. > I'd envisioned the source text for a particular top-level form only being > held for as long as required to eval or compile the form, not being stored > permanently in metadata. > Yes, this occurred to me only after reading your response to Alex. That could be pretty cool, but might require extra specialisation for the macro case. > But I'd have to go and look at the code to see if that's feasible or not. > AOTing would not be a problem since the macroexpansion errors would be > found when the form was AOTed, those errors would never occur at runtime. > Sounds feasible then... Thanks for the kind words about Cursive too, one of my main goals is to > make Clojure more approachable (hence the obsession with error messages), > so I'm glad it's helped your co-workers! And you never know, maybe you'll > like it enough one day to switch :-) > Well the biggest barrier to me trying to use it more are the lack of Emacs & CIDER & Emacs paredit style keybindings. I'm not sure if you can easily share those configs with intellij; but if there was a config that had all that together I'd probably be able to last more than 5 minutes without getting frustrated... A discussion for another thread perhaps... R. > > On 26 August 2016 at 21:15, Rick Moynihan <rick.moyni...@gmail.com> wrote: > >> On 26 August 2016 at 03:11, Colin Fleming <colin.mailingl...@gmail.com> >> wrote: >> >>> Hi Rick, >>> >>> That looks really excellent, and is a huge improvement. Particularly in >>> combination with Leon's proposed change which more precisely identifies the >>> likely failing part of the grammar, this looks like a big win for not much >>> extra effort. >>> >> >> Well it was really just 5-10 minutes work. I think it shows though that >> specs errors are actually better than a lot of people are giving them >> credit for. Once I'd tidied it up a bit it surprised me. >> >> >>> One thing that I think would help a lot would be if it were possible to >>> show the actual text from the failing expression rather than pretty >>> printing a seq representation of it. This would mean modifying the reader >>> such that it either caches the program text from each top-level form as it >>> reads it, or perhaps re-reading the file on an error. This means the >>> relevant code is likely to look more familiar to the user, and also avoids >>> any need to pretty print. Pretty printing is likely to be complicated since >>> it normally works top-down, and uses the type of each form to decide how to >>> lay its sub-parts out. If you're only pretty-printing a fragment from >>> within, say, a large ns form, pprint is unlikely to format it as the user >>> would expect. >>> >> >> Yes, as I was re-rendering the error message it did occur to me that you >> could do lots more to make it even nicer. Capturing the source text is >> certainly one, though mightn't there be a risk of using a large amount of >> memory to store those metadata strings, especially if they're nested, >> overlapping sexps. >> >> Re-reading the file on error would presumably only work if the file was >> available, if it was only aot'd you'd have to either have captured the >> source text at macro expansion time (as mentioned above) or try to lookup >> the source for an even better error - and if it's not found fallback to a >> pretty-pinting of the form. >> >> I didn't want to go too far down the road with the example, as I wanted >> to show how much better the message could be with just a modest amount of >> work. The main ideas of the proposal are also independent and don't rely >> too much on each other. >> >> I'm curious whether the core team plan for the formatting of these >> strings to be a contract; or whether after 1.9.0 is released if they could >> be flagged as experimental - with further improvements to the rendering >> being pushed into future clojure releases? >> >> R. >> >> p.s. Colin, just wanted to say a massive thanks for Cursive! I'm an >> Emacs Cider user myself, but it's really helped many members of our team, >> and I've been so impressed with the progress, that I even asked my boss to >> buy me a copy... I think it'll still take quite a lot to get me off >> Emacs/Cider; but you might well make it! :-) >> >> >>> Cheers, >>> Colin >>> >>> On 26 August 2016 at 12:59, Rick Moynihan <rick.moyni...@gmail.com> >>> wrote: >>> >>>> I think one obvious area that specs error messages could be improved is >>>> with some basic formatting and cosmetic changes. If spec presented errors >>>> not as a wall of text and syntax but with some simple formatting it would >>>> make a big difference to legibility. >>>> >>>> As a starter for 10, why could we not render the messages at a REPL >>>> more like this? (Note this is basically Brian's error - re-rendered): >>>> >>>> user=> (ns such.sequences (require ,,,,)) >>>> >>>> CompilerException clojure.lang.SpecException: >>>> >>>> Call to clojure.core/ns did not conform to fdef [:args] spec >>>> >>>> There was unexpected extra input in: [2] >>>> >>>> with value: (,,, (require [such.vars :as var] >>>> [such.immigration :as immigrate]) >>>> (require midje.checking.checkers.defining >>>> midje.checking.checkers.chatty >>>> midje.checking.checkers.simple >>>> midje.checking.checkers.combining >>>> midje.checking.checkers.collec >>>> tion)) >>>> >>>> Input failed spec predicate: (cat :attr-map (? map?) >>>> :clauses >>>> :clojure.core.specs/ns-clauses) >>>> >>>> When compiling: (such/sequences.clj:1:1) >>>> >>>> user=> >>>> >>>> Some things to point out: >>>> >>>> 1. Provide some extra context by subclassing IllegalArgumentException >>>> as SpecException. This may also help separate SpecException's from other >>>> IllegalArgumentExceptions too, and help tools do something special on a >>>> SpecException. >>>> >>>> 2. Use of new lines to break up and separate text blocks. >>>> >>>> 3. State that it's an fdef spec, and it was the [:args] bit of that >>>> spec that failed. By stating them together we implicitly associate [:args] >>>> with fdef. Note I'm assuming we can also capture that fdef defined this >>>> spec. >>>> >>>> 4. It's a bit unclear what "Extra input" means... so clarify that it >>>> was unexpected. Provide the path [2] as before. >>>> >>>> 5. State the failing value and pretty print it. Note that we also >>>> elide the other passing parameters with a ,,,, >>>> >>>> 6. As before state the predicate that identified the failure in a >>>> humanised manner and the location of the failing form. >>>> >>>> I'm not suggesting these are necessarily good ideas, and I appreciate >>>> I've not considered all of the other cases you might need to, but it seems >>>> that something like the above would be a dramatic if entirely cosmetic >>>> improvement. It would be a shame if clojure.core made no attempt to >>>> humanise the display of these messages and left it entirely up to the >>>> community. >>>> >>>> Thoughts? >>>> >>>> R. >>>> >>>> >>>> On 20 August 2016 at 15:03, Alex Miller <a...@puredanger.com> wrote: >>>> >>>>> >>>>> >>>>> On Saturday, August 20, 2016 at 5:17:59 AM UTC-5, Brian Marick wrote: >>>>>> >>>>>> Yesterday, a bug was filed against Suchwow under 1.9alpha11. It turns >>>>>> out to have been a use of `ns …(require…` instead of `(ns …(:require`. >>>>>> Not >>>>>> in Suchwow, but in Midje. Unfortunately, the Suchwow file the bug report >>>>>> pointed at *also* had that typo - apparently I am prone to it - so adding >>>>>> the colon to the require there didn’t make the problem go away. >>>>>> >>>>>> That caused me to lose my temper and make a fool of myself, which is >>>>>> neither here nor there, except that I apologize to @puredanger. >>>>>> >>>>>> I have two suggestions, though: >>>>>> >>>>>> 1. It has long been the case that Clojure allowed `(ns (require…)` >>>>>> even though that’s strictly incorrect. I suggest that, for backwards >>>>>> compatibility, it be allowed going forward. That is, I think it does no >>>>>> harm for a correct `ns` statement to allow symbols as well as keywords. >>>>>> That wrong code in Midje has been there since Clojure 1.2. >>>>>> >>>>> >>>>> We discussed this before releasing the specs and decided to start on >>>>> the strict side. That said, this is still an alpha and there is plenty of >>>>> time to change our minds prior to official release of 1.9 if that ends up >>>>> being a catastrophic decision. >>>>> >>>>> >>>>>> >>>>>> 2. The following is not a good error message: >>>>>> >>>>>> Exception in thread "main" java.lang.IllegalArgumentException: Call >>>>>> to clojure.core/ns did not conform to spec: >>>>>> In: [2] val: ((require [such.vars :as var] [such.immigration :as >>>>>> immigrate]) (require midje.checking.checkers.defining >>>>>> midje.checking.checkers.chatty midje.checking.checkers.simple >>>>>> midje.checking.checkers.combining midje.checking.checkers.collection)) >>>>>> fails at: [:args] predicate: (cat :attr-map (? map?) :clauses >>>>>> :clojure.core.specs/ns-clauses), Extra input >>>>>> >>>>> >>>>> You left out this next important line too since it points you to >>>>> exactly the file and line where the error occurs: >>>>> >>>>> , compiling:(such/sequences.clj:1:1) >>>>> >>>>> spec produces very detailed error messages driven by the specs and the >>>>> value being validated. I admit that in some cases the output from a spec >>>>> error (particularly for complicated syntaxes where there are wide >>>>> alternative fan-outs) is daunting. However, spec error messages are going >>>>> to be increasingly common for all of us to see and understand and I think >>>>> it is worth taking the time to slow down and actually read them. >>>>> >>>>> > Call to clojure.core/ns did not conform to spec: >>>>> ^^^^^^^^^^^^^^ <- macro that was passed invalid values >>>>> > In: [2] >>>>> ^^^ <- the data path in the :args passed to the macro, here, >>>>> the 2th element is the require clause (ns = 0, such.sequences = 1) >>>>> >>>>> > val: ((require [such.vars :as var] ...) >>>>> ^^ <- the remaining part of the value that did not match (it >>>>> has already matched or "consumed" the first two elements successfully) >>>>> >>>>> > fails at: [:args] >>>>> ^^^^^^ <- the path in the ns fdef spec to the failure >>>>> point >>>>> >>>>> > predicate: (cat :attr-map (? map?) :clauses >>>>> :clojure.core.specs/ns-clauses), >>>>> ^^^etc -> the remaining part of the spec it was >>>>> trying to match >>>>> >>>>> > Extra input >>>>> specs way of saying that it found something (the val above) but it >>>>> wasn't what the spec expected next >>>>> >>>>> >>>>> I'm not trying to pretend this is as easy to digest as an error >>>>> message that would be produced by hand-written validation and error code, >>>>> but it's also notoriously difficult to cover all possible cases (which is >>>>> why the Clojure DSLs have so many error gaps despite having a lot of that >>>>> code). We are looking to decrease the amount of custom error detection and >>>>> reporting, so anything we do has to be something we can do generically. >>>>> >>>>> For the specific case of macroexpanded error reporting, I think there >>>>> *are* more things we can do here (generically) that will improve >>>>> readability. We *know* we are in the context of checking an fdef spec on a >>>>> macro, so some of the ":args" stuff is maybe not necessary. Having the val >>>>> and predicate for a s/cat forwarded to the point of mismatch is both great >>>>> (as it's specific) but also confusing (because parts of both the input and >>>>> the cat spec) are missing which removes important context. I think there >>>>> are ways to indicate that's happening better. Earlier versions of this >>>>> also >>>>> reported the full args and we ended up removing that because in many cases >>>>> it feels redundant (or is potentially large). Maybe there is some >>>>> heuristic >>>>> we could follow on when that would help. >>>>> >>>>> >>>>>> - It would be better to say “`require` should be a keyword, not a >>>>>> symbol” than "fails at: [:args] predicate: (cat :attr-map (? map?) >>>>>> :clauses >>>>>> :clojure.core.specs/ns-clauses), Extra input” >>>>>> >>>>>> - Suggest Clojure.spec error messages follow the “inverted pyramid” >>>>>> structure of news reports: https://en.wikipedia. >>>>>> org/wiki/Inverted_pyramid That would mean the text message about the >>>>>> error would come first, the spec second, and the wrong expression last. >>>>>> >>>>>> - It would be better to name the namespace that has the problem. >>>>>> >>>>> >>>>> As mentioned above, the next line that you omitted points to the file >>>>> and line with the problem (I went to some trouble to ensure that was >>>>> reported properly from the point of use). >>>>> >>>>> >>>>>> - The stack trace adds nothing to the error. If anything, it makes it >>>>>> less understandable, as the sheer amount of text is offputting. >>>>>> >>>>> >>>>> I agree, but this is to some degree an orthogonal issue. Tools >>>>> (including the base REPL) choose what to do when they receive an exception >>>>> from the compiler. I would like to think more carefully about what the >>>>> compiler is producing (informationally) and also how tools should be >>>>> expected to handle these errors. Certainly in cases like this, the stack >>>>> trace is unnecessary and that's true of many compiler errors. That is a >>>>> fixable problem and I am interested in making improvements in this area >>>>> still in 1.9. >>>>> >>>>> >>>>>> My https://github.com/marick/structural-typing does (a small) part >>>>>> of what clojure.spec does. I went to a lot of effort to get it to produce >>>>>> good error messages, and it turned out OK. I doubt it would be applicable >>>>>> to clojure.spec, but I’d be happy to sign over any code that could be of >>>>>> use. >>>>>> >>>>> >>>>> Thanks, I doubt there's much we could use directly. >>>>> >>>>> >>>>>> It’s unfortunate that reporting errors is so much harder than >>>>>> detecting them. >>>>>> >>>>> >>>>> Indeed. Unfortunately errors are read by people and people are hard. >>>>> :) >>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Clojure" group. >>>>> To post to this group, send email to clojure@googlegroups.com >>>>> Note that posts from new members are moderated - please be patient >>>>> with your first post. >>>>> To unsubscribe from this group, send email to >>>>> clojure+unsubscr...@googlegroups.com >>>>> For more options, visit this group at >>>>> http://groups.google.com/group/clojure?hl=en >>>>> --- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Clojure" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to clojure+unsubscr...@googlegroups.com. >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "Clojure" group. >>>> To post to this group, send email to clojure@googlegroups.com >>>> Note that posts from new members are moderated - please be patient with >>>> your first post. >>>> To unsubscribe from this group, send email to >>>> clojure+unsubscr...@googlegroups.com >>>> For more options, visit this group at >>>> http://groups.google.com/group/clojure?hl=en >>>> --- >>>> You received this message because you are subscribed to the Google >>>> Groups "Clojure" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to clojure+unsubscr...@googlegroups.com. >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Clojure" group. >>> To post to this group, send email to clojure@googlegroups.com >>> Note that posts from new members are moderated - please be patient with >>> your first post. >>> To unsubscribe from this group, send email to >>> clojure+unsubscr...@googlegroups.com >>> For more options, visit this group at >>> http://groups.google.com/group/clojure?hl=en >>> --- >>> You received this message because you are subscribed to the Google >>> Groups "Clojure" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to clojure+unsubscr...@googlegroups.com. >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> -- >> You received this message because you are subscribed to the Google >> Groups "Clojure" group. >> To post to this group, send email to clojure@googlegroups.com >> Note that posts from new members are moderated - please be patient with >> your first post. >> To unsubscribe from this group, send email to >> clojure+unsubscr...@googlegroups.com >> For more options, visit this group at >> http://groups.google.com/group/clojure?hl=en >> --- >> You received this message because you are subscribed to the Google Groups >> "Clojure" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to clojure+unsubscr...@googlegroups.com. >> For more options, visit https://groups.google.com/d/optout. >> > > -- > You received this message because you are subscribed to the Google > Groups "Clojure" group. > To post to this group, send email to clojure@googlegroups.com > Note that posts from new members are moderated - please be patient with > your first post. > To unsubscribe from this group, send email to > clojure+unsubscr...@googlegroups.com > For more options, visit this group at > http://groups.google.com/group/clojure?hl=en > --- > You received this message because you are subscribed to the Google Groups > "Clojure" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to clojure+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Clojure" group. To post to this group, send email to clojure@googlegroups.com Note that posts from new members are moderated - please be patient with your first post. To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en --- You received this message because you are subscribed to the Google Groups "Clojure" group. To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
