Skip to content

Resolve string concatenation expressions in field values (#396) - #565

Open
gaoflow wants to merge 2 commits into
sciunto-org:mainfrom
gaoflow:fix-396-resolve-string-concatenation
Open

Resolve string concatenation expressions in field values (#396)#565
gaoflow wants to merge 2 commits into
sciunto-org:mainfrom
gaoflow:fix-396-resolve-string-concatenation

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 23, 2026

Copy link
Copy Markdown

Fixes #396. Three places treated a value that is a # expression as a single token.

Concatenation in an entry field was not interpolated. ResolveStringReferencesMiddleware only handled a value that was one whole reference, so month = 10 # "~" # jan was looked up as a single key and left raw.

A string definition was substituted verbatim. A definition that is itself an expression or another reference was spliced into the field unevaluated, so asiacrypt91name = asiacryptname # "'91" (the case @kmccurley raised in the issue) gave booktitle the value asiacryptname # "'91".

RemoveEnclosingMiddleware took the first and last character of an expression for one enclosing pair. title = "hello" # " " # "world" became hello" # " " # "world, and @string{s = "Asia" # "crypt"} was rewritten as @string{s = {Asia" # "crypt}} — a valid file corrupted by a round trip.

Both middlewares now share one expression scanner, which follows the escaping the splitter applies (a delimiter after a backslash is content). String definitions are resolved up front and iteratively, so chains resolve and a definition on a reference cycle simply never resolves instead of yielding a partial value. A resolved expression is stored brace-enclosed, since unenclosed it would be read back as a reference; an expression that cannot be resolved keeps its exact text and is written back unchanged.

Tests cover the issue repro, definitions that are expressions / chains / cycles, a 500-link chain, and the unresolved-expression round trip. 2588 passed, 12 skipped; full pre-commit suite clean.

@claell published a comparison branch covering several of these cases; credited on the commit, details in a comment below.


This pull request was prepared with the assistance of AI, under my direction and review.

ResolveStringReferencesMiddleware only resolved a field whose entire value
was a single string reference. A concatenation expression such as
`10 # "~" # jan` was looked up as one key, failed, and was left raw
(issue sciunto-org#396); an all-quoted expression like `"a" # "b"` was skipped by
the enclosed-value guard and later mangled by RemoveEnclosingMiddleware.

Detect top-level `#` concatenation (respecting quotes and braces), resolve
each token (number, quoted/braced literal, or string reference) and join the
results. The resolved value is kept enclosed in braces so the downstream
enclosing middlewares treat it as a plain string and it round-trips to valid
BibTeX. If any reference is unknown, the original expression is left
untouched.
@claell

claell commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

I independently explored a broader version of this fix while auditing the parser's round-trip behavior. I have published the result as a comparison branch rather than opening a competing PR:

In addition to resolving ordinary entry-field concatenations, the branch tests and handles:

  • concatenations inside @string definitions;
  • recursively chained string definitions;
  • cyclic definitions without a recursion error;
  • exact preservation of unresolved expressions instead of partial resolution; and
  • preservation of unresolved quoted or braced multi-token expressions through RemoveEnclosingMiddleware.

The branch has two commits so the expression resolver and the enclosing-middleware safeguard can be reviewed independently. Validation is clean: 2584 passed, 12 skipped, plus the complete pre-commit hook suite. Please feel free to compare the implementation or cherry-pick/adapt any of its tests and cases for this PR.

For transparency, this comparison implementation and its tests were prepared with ChatGPT Codex using GPT-5.6 Sol with high reasoning effort. They should still receive normal maintainer review, especially because the work deliberately covers more cases than the original report.

…intact

The remaining places where a value that is a `#` expression is treated as a
single token:

- A string definition was substituted verbatim, so a definition that is itself
  an expression or another reference was spliced into the field unevaluated
  (`asiacrypt91name = asiacryptname # "'91"` from sciunto-org#396). Definitions are now
  resolved up front, iteratively, so chains resolve and a reference cycle simply
  never resolves instead of producing a partial value.
- `RemoveEnclosingMiddleware._strip_enclosing` took the first and last character
  of an expression for one enclosing pair, so an expression bounded by
  delimiters was corrupted, both in a field (`"a" # nope # "b"`, when nothing
  resolves it first) and in a `@string` block, where a valid file was rewritten
  as `@string{s = {Asia" # "crypt}}`.

Both middlewares now share one expression scanner, which follows the escaping
the splitter applies (a delimiter after a backslash is content).

Cases and the comparison branch that prompted them: @claell on sciunto-org#565.

Co-authored-by: Claudius Ellsel <claudius.ellsel@live.de>
@gaoflow

gaoflow commented Jul 26, 2026

Copy link
Copy Markdown
Author

Thanks @claell — the comparison branch was useful and I've folded the cases in, with credit on the commit. Concatenation inside @string definitions, chained definitions, and preservation of an unresolved expression through RemoveEnclosingMiddleware were all real gaps here (every one of your cases failed against this branch), and your enclosing test is in nearly as it was, plus a @string-block row, because @string{s = "Asia" # "crypt"} was being rewritten as @string{s = {Asia" # "crypt}} — a valid file corrupted by a round trip. Two of the five I'd frame differently rather than fold in as stated: "no partial resolution" was already the behaviour here, since an unknown reference already aborted the whole expression (the real gap was the enclosing middleware, i.e. your last point); and nothing recursed before, so a cycle could not raise RecursionError — it did resolve to a wrong value, which is fixed.

I went a different way twice on the implementation. Per-reference recursion trades the cycle case for a new one on chains — @string{s0 = s1 # "x"} 400 links deep raises RecursionError on your branch — so the definition table is resolved iteratively instead, which covers chains and cycles with the same mechanism; and a plain reference to a chained definition (@string{alias = asiacrypt91name} with series = alias) still resolves only one level there, so series and series # "" disagreed. I also kept a single expression scanner shared by both middlewares instead of one in each, using the splitter's own (?<!\\) escape rule, which is what makes values shaped like "sch\"on" # "!" resolve.

2588 passed, 12 skipped (2579 before), full pre-commit suite clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected concatenation of field tokens

2 participants