Hello, Mario Frasca <ma...@anche.no> writes:
> On 13/06/2020 17:18, Nicolas Goaziou wrote: >> Unit tests are not worth a formal definition. However, "test-ox.el" >> contains unit tests. > > I'm not sure what you mean by the first sentence I mean that, even though unit tests are great, and certainly welcome, we first and foremost need a clear definition of a table header. This concept is suggested in the manual, and more accurately defined in ox.el. It would be nice to define it properly in the syntax. > but I found the header tests in the test-ox.el file, and added one > where multiple lines header is accepted. is it related enough as to > be included in my proposal? Sure. > I can move the org-table-collapse-header function from org-table.el to > org-plot.el, but to me it makes little sense, relegating a generic > function to a sub-module: others will look for the functionality in > org-table, not see it, and duplicate the function somewhere else. This doesn't seem to be a generic function, but a very specific one. More on this below. > + (setq glue (or glue " ")) > + (setq max-header-lines (or max-header-lines 4)) Please use `let' instead of `setq' whenever possible, e.g., here. > + (while (equal 'hline (car table)) equal -> eq > + (setq table (cdr table))) (pop table) > + (let* ((trailer table) > + (header-lines (cl-loop for line in table > + until (equal line 'hline) > + collect line > + do (setq trailer (cdr trailer))))) You couldn't resist. This could be extracted as an independent function, which would return the header, or nil. We can also imagine a function returning a cons cell (HEADER . BODY), both HEADER and BODY being list of rows (possibly empty). Note that the function may be more complicated than the above, because, in the following tables | a | | b | or |---| | a | | b | |---| there's a body, but no header. It depends on the definition chosen for headers. > + (if (and trailer (<= (length header-lines) max-header-lines)) > + (cons (apply #'cl-mapcar > + #'(lambda (&rest x) The #', aka, `function', is just noise before `lambda'. --> (lambda (&rest x)) > + (org-trim > + (mapconcat #'identity x glue))) > + header-lines) > + trailer) > + table))) Here you are going further than setting the definition for tables headers. You imply that: | header line 1 | | header line 2 | |---------------| | body | is equivalent to | header line 1 header line 2 | |-----------------------------| | body | I'm not so sure this is a good idea. It might be a good idea for Org Plot, I don't know, but generally speaking, is it? Regards, -- Nicolas Goaziou