-
-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add list package for creating lists #614
Conversation
This is much needed, thank you. “Two times nested” should start with a 3, not an open bullet, shouldn’t it? |
No, that is intentional, but a bad example. Imagine another top level bullet "Third" right before it, then it will make more sense. What you suggest is of course also possible, just without the two lines that I have marked with comment in above code. |
local topLevel = level == 1 | ||
if topLevel then SILE.call("smallskip") else SILE.typesetter:leaveHmode() end | ||
|
||
local numbered = SU.boolean(options.numbered, SU.boolean(options.ordered, options.numbering ~= nil)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mashup of variable names "options.numbered", "options.ordered", and "options.numbering" here combined with nested boolean tests is a bit confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know which one is better. I would pick based on precedent (html - unordered/ordered, tex - ?), but allowing both as an alias could be also fine, users wouldn't have to remember it.
In the grand scheme of things, it could be useful to implement a system which warns for unused options and possibly knows and suggests valid options. But right now, if I specify for example \list[unordered=false]
, it won't work and I will get no warning. So if I don't notice incorrect output, I am out of luck. But it wouldn't be hard to do warning like:
! Option
unordered
is not used. Did you mean:ordered
,numbering
orbullet
?
That is obviously completely out of scope for this PR, but I could contribute it sometime in the future, if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obscure, not understandable.
end) | ||
|
||
SILE.registerCommand("list:item", function (options, content) | ||
local settings = SILE.scratch.listStack[#SILE.scratch.listStack] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a global. What happens if one has, says, for the mere example a footnote in list, and a list in the footnote? A mess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And what is expected if one mistakenly calls this list:item outside a list.
SILE.settings.declare({ | ||
name = "list.bullet.inset", | ||
type = "Length", | ||
default = SILE.length.parse("1.3em"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for bullets and numbering. Ever tried it with roman numerals?
end) | ||
SILE.scratch.listStack[#SILE.scratch.listStack] = nil | ||
|
||
if topLevel then SILE.call("smallskip") else SILE.typesetter:leaveHmode() end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, a hard-coded smallskip. Explain. Now set a parskip and explain the expected behavior.
SILE.scratch.listStack[#SILE.scratch.listStack] = nil | ||
|
||
if topLevel then SILE.call("smallskip") else SILE.typesetter:leaveHmode() end | ||
SILE.call("noindent") -- list does not create a new paragraph |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah? They do not create a new paragraph after them? Explain why.
\command{\\list:item} has option \code{number}, to override the number of the item in case of numbered lists. | ||
By default, the numbered list starts at 1 and each item has a number that is one bigger than of the previous one. | ||
|
||
Lists can be arbitrarily nested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proof? When the documentation doesn't show it, and there are not a single test case, am I to believe it?
\code{bullet} a string with explicit bullet character to use. | ||
\code{numbering} a string denoting the numbering system to use for ordered lists. | ||
See the documentation of counters for possible values. | ||
\code{lskip} width reserved for the bullets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list seems to use options.rskip
(but should it?). Maybe I missed it above, but I didn't see any any options.lskip
...
SILE.settings.declare({ | ||
name = "list.bullet.numbered", | ||
type = "string", | ||
default = ".", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it's a dot. Weird name then, list.bullet.numbered
(you are numbering bullets as dots? Heh.)
local baseLSkip = SILE.settings.get("document.lskip") | ||
baseLSkip = baseLSkip and baseLSkip.width or SILE.length.new() | ||
SILE.settings.temporarily(function () | ||
SILE.settings.set("document.lskip", SILE.nodefactory.newGlue({width = baseLSkip + inset})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does it look if the paragraph before the list has a parindent > inset ? ;)
Some good points, but I think the implementation is not robust enough. I added a few comments, but I don't ask for them to be fixed here. I wanted to see how my own implementation (which I will propose at one point) compare to this, and if there were ideas I had possibly missed. Overall, I think we can do better. See #1365 |
I think this could now be closed without merge, since we got #1365 instead - With obviously many thanks to @alerque for the early casile attempt mentioned above and to @Darkyenus for this very PR - You were, how do we say, pioneers or precursors here! Looking at this code obviously helped a lot in reaching #1365 targets. |
Partially based on casile/lists.lua.
Supports:
Not done:
Current output: