quarto-dev / quarto-cli

Open-source scientific and technical publishing system built on Pandoc.
https://quarto.org
Other
3.64k stars 298 forks source link

Code listings and "filename" cause error for PDF files #9243

Open Steinthal opened 4 months ago

Steinthal commented 4 months ago

Bug description

When setting "listings: true" for format PDF, the div parameter "filename" causes an error:

Package keyval Error: filename undefined.

See the keyval package documentation for explanation.
Type  H <return>  for immediate help.
 ...                                              

l.151 \begin{lstlisting}[filename=Terminal]

Using "title" instead of "filename" does not throw the error. However, "title" does not produce the code listings heading in html.

Steps to reproduce

---
title: "Test Listings"

format: 
  pdf:
    listings: true
---

## Test with listings

```{.zsh filename="Terminal"}
echo "Hello World"

more text



### Expected behavior

* "filename" should be used as title for the code 

### Actual behavior

* The PDF compilation stops with an error

### Your environment

- IDE: Version 2023.12.1+402 (2023.12.1+402)
- Quarto: version 1.4.552
- macOS: Sonoma 14.4.1

### Quarto check output

_No response_
mcanouil commented 4 months ago

It seems to be an issue with the Lua filter to decorate the code blocks: https://github.com/quarto-dev/quarto-cli/blob/f2e95a6a69e5cb005bf845cbfd2326fd606087d7/src/resources/filters/customnodes/decoratedcodeblock.lua

Note that it was working in 1.3.450, so it's a regression here.

cscheid commented 1 month ago

Note that it was working in 1.3.450, so it's a regression here.

It was "working" in the sense that it didn't crash. But it didn't work in the sense that there's no filename (or title) in the output.

cscheid commented 1 month ago

There's more than one thing going wrong with filename="" in code listings in PDF output:

## Test with listings

```{#lst-1 .zsh lst-cap="A caption" filename="Terminal"}
echo "Hello World"

See @lst-1.



This works in HTML but crashes in PDF, independent of `listings: true` or not. The div syntax also doesn't work.
cscheid commented 1 month ago

The problem is deeper. We've regressed on this, in at least two ways:

---
format: pdf
keep-tex: true
---

```{#lst-1 .python lst-cap="A caption"}
print("Hello, World!")


v1.3.450 produces this:

<img width="803" alt="image" src="https://github.com/quarto-dev/quarto-cli/assets/285675/e9ce11da-1179-4929-9bf8-b5c1f098fdfb">

main produces this:

<img width="767" alt="image" src="https://github.com/quarto-dev/quarto-cli/assets/285675/41759c78-ebde-45c2-b89b-9e42a15476d9">

Note the lack of all highlighting. The first fix is to forward the classes from the FloatRefTarget to the listing itself. But then, we get this:

<img width="776" alt="image" src="https://github.com/quarto-dev/quarto-cli/assets/285675/53c50f79-e25d-421b-971a-055583edabe5">

What's happening here is that `main` is not overriding Pandoc's definition of the Shaded environment, compared to 1.3. This check happens in `meta.lua`:

https://github.com/quarto-dev/quarto-cli/blob/7d207f6ab36afcca0dac92a270193dcefef88b54/src/resources/filters/layout/meta.lua#L38

On `main`, I found both `useCodeBlockBorder` and `useCodeBlockBg` to be false, by inspection.

On `v1.3.450`, though, `useCodeBlockBorder` is true, because `param('adaptive-text-highlighting', false)` is true. This parameter is set on `layout.ts`. On https://github.com/quarto-dev/quarto-cli/commit/136e702a2cc750f374f4281f1299f9fa6f3b789f, we changed the behavior and `adaptive-text-highlighting` became false. I think this is actually a bug fix, because `adaptive-text-highlighting` appears to be about themes that can be dark or light, and the PDF format simply doesn't support that.

I think my conclusion after all of this is that `main` _shouldn't_ override the definition of Shaded in this simple case.
cscheid commented 1 month ago

In 1.3.450,listings: true barely worked. For example, it couldn't be used to create cross-referenceable code listings either:

---
title: "Test Listings"
keep-tex: true
format: 
  pdf:
    listings: true
---

## Test with listings

```{#lst-1 .zsh lst-cap="Hello"}
echo "Hello World"

more text, see @lst-1.


This doesn't work in 1.3.450:

```tex
% invalid output from Quarto
\begin{lstlisting}[lst-cap=Hello, label=lst-1]

lstlisting doesn't appear to offer syntax highlighting, either. I know that Quarto nominally supports it and I don't intend on breaking this behavior, but I honestly don't want to invest a lot of engineering time on output that never worked that well to begin with, and for which we have better alternatives.

I'll add a PR for the Shaded fixes, but I'm not going to attempt to make lstlisting work with the rest of Quarto in this PR. I think we should recommend people not to use listings: true and eventually deprecate that option.