WA, USA
tryin' to catch the last train out of Omelas
Comment by ~technomancy on ~technomancy/fennel
Thanks! I'm focusing on getting the next Fennel release out first and then I'll take a look at this and the other one.
Comment by ~technomancy on ~technomancy/fennel
REPORTED
RESOLVED CLOSEDbug added by ~technomancy on ~technomancy/fennel
Comment by ~technomancy on ~technomancy/fennel
The root cause of this problem is that
(length ast)
behaves unpredictably on sparse tables.I think it's a good idea when writing macros to avoid sparse tables. However, we could check for these immediately following macroexpansion and replace the nils in any list with
(utils.sym :nil)
and that should solve the problem, since it is easy for sparse tables to sneak in by accident.
Comment by ~technomancy on ~technomancy/fennel
Looking back at this, the root cause of the problem is a misunderstanding of the macro scope. The whole pattern of requiring fennel in order to get
fennel.view
is unnecessary because the same function is provided asview
in macro scope. The problem is that this is not obvious; everyone is used to getting the function thru the fennel module.But the fact that
(require :fennel)
even works at all from inside a macro is ... well, it's a leaky implementation detail. We use it because of the way docstrings work; the compiled output assumes that it can set metadata withrequire("fennel").metadata
. So we include just the minimum shim to allow that; it's not really the fennel module.The problem is that it's not at all obvious that this is the case.
(require :fennel)
looks like it works, and it's the expected way that people normally access the view function outside macro scope.I can't think of a good way to make this clear to people, unless they read the reference very thoroughly. Because of this I'm inclined to let just view thru, and omit the rest, because it's obvious there are better ways to construct symbols and lists in compiler scope.
Comment by ~technomancy on ~technomancy/fennel
Good catch; thanks.
Fixed in 58ed229.
REPORTED
RESOLVED CLOSEDComment by ~technomancy on ~technomancy/fennel
So I guess the question here is how much of the main fennel module do we want to expose inside the sandbox. Of course, nearly everything here which doesn't violate the sandbox is redundant because most of it is already available as a global. But wanting to put it on the module is reasonable IMO because that's the first place you'd look for
view
. But maybe not for the other functions likelist
,sym
, etc?{:metadata compiler.metadata :view view :sym-char? utils.sym-char? :list utils.list :list? utils.list? :sym utils.sym :sym? utils.sym? :sequence utils.sequence :sequence? utils.sequence? :varg utils.varg :varg? utils.varg? :comment utils.comment :comment? utils.comment?}Here's what I'm thinking, but maybe it's overkill? We could just add
view
too; that would also be reasonable. Not sure what inclusion criteria is best here.
Comment by ~technomancy on ~technomancy/fennel
This was merged!
REPORTED
RESOLVED FIXEDComment by ~technomancy on ~technomancy/fennel
I've got the patch for this up here if folks want to take a look https://lists.sr.ht/~technomancy/fennel/patches/33716
Comment by ~technomancy on ~technomancy/fennel
I've got some code I'm working on that includes byte-based column offsets in the AST nodes.
I can extend it to make the error messages use it, but I figure I'd see if you had done anything on this first.