refactoring, cleanup + found the old waitNested bug...

Signed-off-by: Alex A. Naanou <alex.nanou@gmail.com>

v3/pwiki/parser.js

@@ -23,11 +23,7 @@ var pwpath = require('./path')
 // XXX TODO:
 // 		callbacks on elements resolving...
 // XXX ASAP move the macros here...
-// XXX should we warn about stuff like <macro src=/moo/> -- currently 
-// 		this will simply be ignored, i.e. passed trough the parser 
-// 		without change...
-// XXX might be a good idea to both think of a good async parse and
-// 		create tools for sync parsing (get links etc.)...
+// XXX BUG?: <macro src=/moo/> is not parsed correctly...
 // XXX need to correctly handle nested and escaped quotes...
 // 		i.e.
 // 			"aaa \"bbb \\"ccc\\" bbb\" aaa"
@@ -155,18 +151,6 @@ module.BaseParser = {
 
 	// helpers...
 	//
-	normalizeFilters: function(filters){
-		var skip = new Set()
-		return filters
-			.flat()
-			.tailUnique()
-			.filter(function(filter){
-				filter[0] == '-'
-					&& skip.add(filter.slice(1))
-				return filter[0] != '-' }) 
-			.filter(function(filter){
-				return !skip.has(filter) })},
-	//
 	// Spec format:
 	// 	[<orderd>, ... [<keyword>, ...]]
 	//
@@ -271,12 +255,26 @@ module.BaseParser = {
 				] })
 			.flat() },
 
+	normalizeFilters: function(filters){
+		var skip = new Set()
+		return filters
+			.flat()
+			.tailUnique()
+			.filter(function(filter){
+				filter[0] == '-'
+					&& skip.add(filter.slice(1))
+				return filter[0] != '-' }) 
+			.filter(function(filter){
+				return !skip.has(filter) })},
 	applyFilters: function(filters, str, state={}){
 		var that = this
 		filters = this.normalizeFilters(filters)
 			.filter(function(f){
 				return f in (that.filters ?? {})})
 		var handle = function(str){
+			// skip non-basic data...
+			if(typeof(str) == 'object'){
+				return str }
 			return filters
 				.reduce(function(res, filter){
 					return that.filters[filter].call(that, str, state) }, str) }
@@ -324,15 +322,11 @@ module.BaseParser = {
 		str = typeof(str) != 'string' ?
 			str+''
 			: str
-		// XXX we can't get .raw from the page without going async...
-		//str = str 
-		//	?? page.raw
 		// NOTE: we are doing a separate pass for comments to completely 
 		// 		decouple them from the base macro syntax, making them fully 
 		// 		transparent...
 		str = this.stripComments(str)
 
-		// XXX should this be cached???
 		var macro_pattern = this.MACRO_PATTERN 
 			?? this.buildMacroPattern(Object.deepKeys(this.macros))
 		var macro_pattern_groups = this.MACRO_PATTERN_GROUPS 
@@ -350,7 +344,6 @@ module.BaseParser = {
 				// 		second time here, this gives us access to named groups
 				// 		avoiding maintaining match indexes with the .split(..) 
 				// 		output...
-				// XXX for some reason .match(..) here returns a list with a string...
 				var cur = [...match.matchAll(macro_pattern)][0].groups
 				// special case: escaped inline macro -> keep as text...
 				if(match.startsWith('\\@')){
@@ -484,7 +477,6 @@ module.BaseParser = {
 
 			// assert nesting rules...
 			// NOTE: we only check for direct nesting...
-			// XXX might be a good idea to link nested block to the parent...
 			if(this.macros[value.name] instanceof Array
 					// stray nesting...
 					&& (context 
@@ -531,13 +523,13 @@ module.BaseParser = {
 			// normal value...
 			yield value } }, 
 
+	// Generate ast...
+	//
+	// NOTE: this is a convenience wrapper of .group(..), for more docs 
+	// 		see it...
 	// NOTE: the output of this can be safely cached, it does not depend
 	// 		on anything external and as long as the code stays the same 
 	// 		this will not change.
-	// XXX do we need a pre-parse stage???
-	// 		- expand local macros
-	// 		- collect links
-	// 		- ...
 	ast: function(...args){
 		return [...this.group(...args)] },
 
@@ -573,6 +565,8 @@ module.BaseParser = {
 	//		...
 	//	}
 	//
+	// NOTE: this is always sync, but some of the items in the returned 
+	// 		array may be promises.
 	// NOTE: .waitNested and .waitAll are "live", i.e. while expanding,
 	//		at any given time they contain the promise of the last async
 	//		element upto the point of read. .wait however is set at the 
@@ -606,16 +600,6 @@ module.BaseParser = {
 	//				states and the like, async/await can't...
 	// XXX Q: do we need generators?
 	// XXX Handle errors...
-	// XXX this needs a careful rewrite of the .macros.* for the new scheme:
-	// 		- expand
-	// 		- "merge"
-	// 			a rendering API: a set ov events/callbacks allowing both 
-	// 			sync (text) and async (DOM) rendering
-	// 			in the simplest form: take the expanded AST and merge 
-	// 			into a single string
-	// XXX BUG: .wait can resolve before everything in the tree is resolved...
-	// 		to reproduce:
-	// 			@include(/async/recursive/SelfOther recursive="recursion found")
 	expand: function(page, ast, state={}, nested_handlers={}){
 		var that = this
 		ast = typeof(ast) != 'object' ?
@@ -661,7 +645,6 @@ module.BaseParser = {
 				continue }
 
 			// expand down...
-			// XXX cache this as an AST
 			body 
 				&& !that.macros[name].lazy
 				&& (body = this.expand(page, body, state, nested_handlers))
@@ -729,7 +712,7 @@ module.BaseParser = {
 
 		return elems },
 
-	// resolve stage II macros and merge results...
+	// Resolve macros (stage II)...
 	//
 	//	<ast> ::= [ <item>, ... ]
 	//	<item> ::=
@@ -762,13 +745,14 @@ module.BaseParser = {
 				// exec stage II macros...
 				if(typeof(elem.value) == 'function'){
 					let e = elem
+					let func = e.value
 					Promise.awaitOrRun(
 						// if not everything is resolved, delay the stage II
 						// callbacks till .wait is done...
 						state.wait,
 						function(){
 							return elem = e.value = 
-								e.value(state) }) 
+								func(state) }) 
 					break } 
 				elem = elem.value }
 			if(elem == null){
@@ -808,20 +792,22 @@ module.BaseParser = {
 				return false } }
 		return true },
 
-	// XXX render api...
-	// XXX how should this play with filters???
-	// 		...should filters be client-side only??
-	render: function*(page, ast, callback, state={}){
-		// XXX
-	},
+	filters: undefined, 
 
-	// merge stage III...
+	// Merge and apply global filters (stage III)...
 	//
 	// - ensure the ast is fully resolved
 	// - apply nested stage III handlers
 	// - apply filters on whole ast
 	//
 	//
+	// XXX RECURSION might be a good idea to limit recursion/nesting depth 
+	// 		of inner merge(..)...
+	// XXX to allow nested filter blocks to self-exclude form global/uppaer
+	// 		filters would be nice to be able to do the filtering before
+	// 		we are fully resolved, i.e. after all the promises but before 
+	// 		all the functions are gone from the ast...
+	// 		...not yet sure how to get this through the merge(..)
 	merge: function(page, ast, state={}, nested_handlers={}, wait='wait'){
 		var that = this
 
@@ -832,7 +818,6 @@ module.BaseParser = {
 						e.call(that, state)
 						: e })
 				.flat() }
-		// XXX might be a good idea to limit recursion depth here...
 		var merge = function(ast){
 			return Promise.awaitOrRun(
 				...(state.unresolved ?? []),
@@ -868,18 +853,24 @@ module.BaseParser = {
 
 	exec: function(page, ast, state={}, nested_handlers={}, wait='wait'){
 		return this.merge(...arguments) },
+	execNested: function(page, ast, state={}, nested_handlers={}){
+		//* XXX waitNested here deadlocks the parser -- not sure why...
+		return this.exec(page, ast, state, nested_handlers, 'waitNested') },
+		/*/
+		return this.exec(page, ast, state, nested_handlers, 'unresolved') },
+		//*/
 
 
+	// XXX render api...
 	// XXX how should this play with filters???
 	// 		...should filters be client-side only??
-	execNested: function(page, ast, state={}, nested_handlers={}){
-		return this.exec(page, ast, state, nested_handlers, 'waitNestedi') },
+	render: function*(page, ast, callback, state={}){
+		// XXX
+	},
 }
 
 
 // XXX do we need anything else like .doc, attrs???
-// XXX might be a good idea to offload arg value parsing to here...
-// XXX should macros be lazy by default???
 var Macro =
 module.Macro = 
 function(spec, func){
@@ -893,6 +884,7 @@ function(spec, func){
 	return func }
 
 
+// wait for .waitNested
 var isolated =
 module.isolated = 
 function(macro){
@@ -923,7 +915,11 @@ var parser =
 module.parser = {
 	__proto__: BaseParser,
 
-	INCLUDE_NEST_LIMIT: 20,
+	// String to be substetuted for a recursive include...
+	//
+	// NOTE: if set to null include will throw an error if recursion is 
+	// 		detected.
+	RECURSION_STRING: '',
 
 	// XXX should this be here, in page, or both?
 	filters: {
@@ -958,20 +954,20 @@ module.parser = {
 		// 		| -<filter> <filter-spec>
 		//
 		// XXX need a way to exclude some filters in some nested locks...
-		// XXX BUG: RECURSION: this breaks @include(..)'s recursion tests...
+		// XXX LOCAL_FILTERS do we combine local filters with state filters???
 		filter: function(page, args, body, state){
 			var that = this
 
+			// get filters...
 			delete args.text
 			delete args.body
 			var filters = Object.keys(args)
 
 			// local filter...
 			if(body){
-				// stage I
-				body = this.expand(page, body, state)
-
 				// stage II
+				// NOTE: stage I is handled by .expand(..) as we are not 
+				// 		lazy(..)'ied...
 				return function(state){
 					body = that.resolve(page, body, state)
 
@@ -981,9 +977,8 @@ module.parser = {
 							that.merge(page, body, state),
 							function(body){
 								// apply the filters...
-								// XXX combine with state.filters???
+								// XXX LOCAL_FILTERS combine with state.filters???
 								return that.applyFilters(filters, body, state) }) } } 
-
 			// global filter...
 			} else if(filters.length > 0){
 				(state.filters = (state.filters ??= []))
@@ -1283,30 +1278,28 @@ module.parser = {
 		// 		the context, this potentially can lead to false positives.
 		//
 		// XXX FILTER do we skip includes from outer filters???
-		// XXX add path recursion test to data -- fail if two paths resolve 
-		// 		to the same context...
 		// XXX need a way to make encode option transparent...
+		// XXX RECURSIVE do we set state.recursive or keep it local???
+		// 		...i.e. should it be inherited??
 		// XXX do we want to load a specific slot/block???
-		// XXX REVISE...
+		// XXX page API used:
+		// 		.resolvePathVars(path)
+		// 		.get(path)
+		// 			is this a promise/value, iterable promise a generator
+		// 			an async generator, ... or a combination/stack of the above???
 		include: Macro(
 			['src', 'recursive', 'join', 
 				['s', 'strict', 'isolated']],
-			// XXX need a wrapper protocol -- is this the level for it???
-			// XXX page API used:
-			// 		.resolvePathVars(path)
-			// 		.get(path)
-			// 			is this a promise/value, iterable promise a generator
-			// 			an async generator, ... or a combination/stack of the above???
 			lazy(
 			function(page, args, body, state, handler){
 				var that = this
 
+				// XXX RECURSIVE de we inherit this???
 				var recursive = 
 				state.recursive = 
 					args.recursive 
 						?? state.recursive
-				// XXX get default...
-				recursive ??= ''
+						?? this.RECURSION_STRING
 
 				var base = page.basepath
 				return Promise.awaitOrRun(
@@ -1314,47 +1307,30 @@ module.parser = {
 					function(src){
 						//src = page.resolvePathVars(src)
 
-						// XXX should this be a tree??
-						// 		...need to at least split direct and 
-						// 		indirect dependencies...
-						// XXX would be nice to separate direct (in-page) 
-						// 		depenedencies nad nested...
-						// XXX do we need the same for real paths???
-						// 		no, because actual paths are meaningless 
-						// 		out of context...
-						var depends = ((state.depends ??= {})[src] ??= {})
+						// XXX is this a good place for this -- maybe someplace more global???
+						// XXX is there an inline way to do this???
+						var isRecursive = function(page, tree, path=[]){
+							if(!(page instanceof Array)){
+								if(path.includes(page)){
+									return path }
+								return page in tree
+									&& isRecursive(Object.keys(tree[page]), tree, [...path, page]) }
+							for(var p of page){
+								if(p = isRecursive(p, tree, path)){
+									return p } }
+							return false }
 
 						// content handler...
 						handler ??= 
 							function(page, body, path, text, state){
-								// re-include limit...
-								//* XXX RECURSION
-								// XXX HACK???
-								if( ++(state.included ??= {[path]: 0})[path] 
-										> this.INCLUDE_NEST_LIMIT ?? 20){
-									// XXX BUG: for some reason for async recursion this
-									// 		breaks returning [object Object] overriding
-									// 		the actual return value
-									if(!recursive){
-										throw new Error(path +': include limit reached: '+ state.included[path]) }
-									return that.expand(page, recursive, state) }
-								// handle recursion...
-								// XXX BUG: for some reason this does not work for async...
-								// 		...and works quite differently in tests and 
-								// 		in console -- returns [object Object] in the
-								// 		former and hangs in the later...
- 								var include_stack = state.include_stack ??= []
-								if(include_stack.includes(path)){
-									if(!recursive){
-										throw new Error('Recursive macro: '+include_stack) }
-									return that.expand(page, recursive, state) }
-								include_stack.push(path)
-								//*/
 
 								// XXX check cache???
 
+								// XXX do we include in page or page.get(src)'s context????
+								page = page.get(path)
+
 								var nested
-								var res = args.isolated ?
+								return args.isolated ?
 									//this.resolve(
 									this.merge(
 										page, 
@@ -1364,26 +1340,26 @@ module.parser = {
 											: {})
 									// XXX FILTER need to localize target page 
 									// 		filters to it, somehow...
-									: this.expand(page, text, state)
-
-								//* XXX RECURSION
-								// handle recursion...
-								Promise.awaitOrRun(
-									(nested ?? {}).waitAll,
-									state.waitAll,
-									function(){
-										state.include_stack.at(-1) == src
-											&& state.include_stack.pop()
-										// cleanup...
-										if(state.include_stack.length == 0){
-											delete state.include_stack
-											delete state.recursive } }) 
-								//*/
-
-								return res }
+									// XXX should this be .merge(..)???
+									// 		.merge(..) here breaks things...
+									: this.expand(page, text, state) }
 
 						var pageHandler =
 							function([path, text]){
+								// recursion...
+								var included = state.included ??= {}
+								var cur = included[path] ??= {}
+								cur[page.path] ??= true
+								// check...
+								var p = isRecursive(path, included)
+								if(p !== false){
+									if(recursive == null){
+										throw new Error('Recursion detected:'
+											+'\n\t'+ [...p, path].join('\n\t -> ')) }
+									// set error data...
+									;(state.vars ??= {})['error:recursion'] = path
+									return that.expand(page, recursive, state) }
+
 								// handle nested promises...
 								return Promise.awaitOrRun(
 									text,
@@ -1395,6 +1371,10 @@ module.parser = {
 							page.get(src).matched,
 							page.get(src).raw,
 							function(paths, texts){
+								// track pattern matches...
+								if(paths.length > 1){
+									;(state.matches ??= {})[page.path +'|'+ src] = paths }
+
 								texts = 
 									// XXX how do we handle paths returning non-strings???
 									// special case: list page...
@@ -1481,9 +1461,9 @@ module.parser = {
 		// 		macro with one exception, when used in quote, the body is 
 		// 		not expanded...
 		// NOTE: the filter argument uses the same filters as @filter(..)
+		//
 		// XXX might be a good idea to do an auto-filter that would be 
 		// 		apropriately selected according to format -- md, html, ...
-		// XXX filter...
 		quote: Macro(
 			['src', 'join', 'filter'],
 			quoting(

v3/pwiki/test/parser.js

@@ -17,11 +17,15 @@ module.exports.PAGES = {
 	'/page': 'Page',
 	'/async/page': Promise.resolve('Page'),
 	'/includePage': '@include(/page)',
+	'/includePagePage': '@include(/page) @include(/page)',
 	'/isolated': '@slot(slot original)',
 
 	'/recursive/Self': '<< @include(/recursive/Self) >>',
 	'/recursive/OtherSelf': '<< @include(/recursive/SelfOther) >>',
 	'/recursive/SelfOther': '<< @include(/recursive/OtherSelf) >>',
+	'/recursive/OtherOtherSelf': '<< @include(/recursive/OtherSelfOther) >>',
+	'/recursive/OtherSelfOther': '<< @include(/recursive/SelfOtherOther) >>',
+	'/recursive/SelfOtherOther': '<< @include(/recursive/OtherOtherSelf) >>',
 	'/async/recursive/Self': Promise.resolve('<< @include(/async/recursive/Self) >>'),
 	'/async/recursive/OtherSelf': Promise.resolve('<< @include(/async/recursive/SelfOther) >>'),
 	'/async/recursive/SelfOther': Promise.resolve('<< @include(/async/recursive/OtherSelf) >>'),
@@ -270,6 +274,11 @@ test.Setups({
 		return this.include(assert, '/page') },
 	include_include_page: function(assert){
 		return this.include(assert, '/includePage', 'Page') },
+	// XXX this triggers recursion guard...
+	include_include_page_page: function(assert){
+		return this.include(assert, 
+			'/includePagePage', 
+				'Page Page') },
 	include_async: function(assert){
 		return this.include(assert, '/async/page', 'Page') },
 	// islotaed...