👌 IMPROVE: Support for keeping abbreviations at enclosing values.

[lartpang]

@drakeguan @jagd @mgorny @sjpfenninger

In this commit, I add a new argument keep_abbr_string into AddEnclosingMiddleware to keep the abbreviation (@string) when enclosing the value and not resolving the string.

[MiWeiss]

Suggested change

	value: tuple,
	value: tuple[str,bool],

[MiWeiss]

Hi @lartpang

Thanks a lot for your PR.

This is certainly a feature we want to support. However, there's two major comments to address:

• In principle, I try to keep a match between "Parse" and "Write" Middleware, where the latter (partially) reverts the first. In your case: The stings were dereferenced in ResolveStringReferencesMiddleware, thus they should be put back in a ResolveStringReferencesMiddleware (in interpolate.py). I see how that's not very straight forward (as its very connected to the enclosings) - but please try to consider moving your solution in that direction. Then, if you absolutely can't find a nice solution, just say so. I'll have a look and re-evaluate (I'm a bit short on time atm).
• Make sure the test suite covers end-to-end cases, i.e., cases where the ResolveStringReferencesMiddleware works together with your middleware changes. That's important to protect from regressions, as the two are very much tied together.

I'll check for minor things after we settled on these, but I don't expect any hickups after that :-)

Thanks again

[lartpang]

@MiWeiss Thanks for your review.

In principle, I try to keep a match between "Parse" and "Write" Middleware, where the latter (partially) reverts the first. In your case: The strings were dereferenced in ResolveStringReferencesMiddleware, thus they should be put back in a ResolveStringReferencesMiddleware (in interpolate.py). I see how that's not very straight forward (as its very connected to the enclosings) - but please try to consider moving your solution in that direction. Then, if you absolutely can't find a nice solution, just say so. I'll have a look and re-evaluate (I'm a bit short on time atm).

ResolveStringReferencesMiddleware is used to resolve string objects in bibtex, which seems to conflict with the feature of retaining string objects in this commit:
Because if ResolveStringReferencesMiddleware is used, it explicitly indicates that string objects should be resolved, so there would be no need to emphasize "retaining string objects" additionally.
The requirement to "retain string objects" only exists when enclosing values (maybe), which seems difficult to decouple from the operation of AddEnclosingMiddleware.