sile-typesetter / sile

The SILE Typesetter ā€” Simonā€™s Improved Layout Engine
https://sile-typesetter.org
MIT License
1.64k stars 96 forks source link

[regression] `-m` now requires an argument be set, contrary to documentation #1224

Open ctrlcctrlv opened 3 years ago

ctrlcctrlv commented 3 years ago
[fred@šŸ‡č‘”萄šŸ‡PativUnicodePaper]$ sile pativ.sil -o pativ.pdf -m
option -m requires a value to be set

This broke my Makefile, which I based on one of @alerque'sā€¦

https://github.com/ctrlcctrlv/TaiwaneseKanaUnicodePaper/blob/e7b30c3a08a8bb15f131d0c1194ea9098811721f/Makefile#L14

The documentation still states:

[fred@šŸ‡č‘”萄šŸ‡PativUnicodePaper]$ man sile.1
       -m, --makedeps[=file]
              Generate  a  list of dependencies in Makefile format.  If no filename is specified, the input filename with .d appended will
              be used.

And I do have proof that this used to work because I do see a pativ.d fileā€¦

image

alerque commented 3 years ago

I just fiddled with this a bit and it looks like it might actually be a bug in lua_cliargs. We seem to have regressed on this moving from 2.3ā†’3.0. So far I haven't figured out how to get the defaults behavior to work as advertised.

ctrlcctrlv commented 3 years ago

Interesting. I was literally just typing "I've looked at the code and can't figure out what we broke".

alerque commented 3 years ago

It looks like the entire paradigm shifted in 3.x from the meaning of "optional" being "supplying an argument to this option is optional" to "supplying this argument at all is optional".

What we have documented (and did previously work) is the former, but the toolkit currently only seems to support the latter.

Given the known ambiguities of also dealing with the "splat" parse (which we use for the input without an option flag) I suspect our best course of action is to switch our documentation to the actual supported paradigm.

If that is prohibitively hard to use, we could split a flag for making the deps file from a second option with a required value for changing the default name to write to.

alerque commented 3 years ago

https://github.com/amireh/lua_cliargs/issues/69

alerque commented 3 years ago

I suppose the other solution here would be to do some type casting foolery and accept false (default to off), true (append .d to input filename, or string (user supplied string). I kind of hate magic type casting like that though.

ctrlcctrlv commented 3 years ago

There's actually another option. We already write a bunch of files without asking the user for permission based on the name of the input .sil file. We could drop the ability to set another filename to write to at all and move it to a SILE configuration parameter that they need to set in their document if they want to change it.

ctrlcctrlv commented 3 years ago

(To be clear, if we can't get back the old behavior my idea is what I prefer be done.)

alerque commented 3 years ago

That could work too, but only in conjunction with a new option of a directory of where to write such files, otherwise it is going to kill CaSILE with it's dedicated build directory for all intermediate build artifacts and scratch stuff.

But honestly I'd rather go the other way. The .toc file we write is already a sore point with me. For example while building the website I had to hack around it getting published along with examples because Jekyll doesn't understand that some random untracked files lying around might be irrelevant.

ctrlcctrlv commented 3 years ago

Why can't CaSILE set the option?

alerque commented 3 years ago

@ctrlcctrlv Because with your suggestion there wouldn't even be a way to set the option to a location not in the current directory, it would be hard coded to write a file in a space it isn't allowed to touch.

ctrlcctrlv commented 3 years ago

Why can't you set the option to a relative or full path?

alerque commented 3 years ago

My apologies, I misread your suggestion. I read the part about dropping the ability to set a filename and somehow missed the proposal to add an in-document option.

Making the location for all scratch file writes something that could be configured by an external library might be a solution. That could be used be -e presumably too. It seems more convoluted than it needs to be though. Then again the solution I want is dependent on a CLI parsing option we don't actually have access to right now.

ctrlcctrlv commented 3 years ago

I don't think it's more convoluted than it needs to be, and as a bonus solves a longstanding issue with the unmovable *.toc files. If you ask me, everybody's happy with my solution, while yours leaves too many loose ends. ;-)