Skip to content
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

Rename placehoder "sheetsize_default" to <!-- %template_sheetsize% --> #380

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

kvid
Copy link
Collaborator

@kvid kvid commented Jun 9, 2024

@formatc1702 must must check if this suggested change (placeholder name consistency) is OK, or if it might have some bad side-effects. If there is a good reason to keep this one placeholder very different from the others, then that reason should be explained in a code comment.

A bonus advantage by using the name <!-- %template_sheetsize% -->, is that it will be easy to later generalize by letting any string or numeric metadata.{item}.{key} entry replace the corresponding <!-- %{item}_{key}% --> placeholder.

TODO: If both PR #371 and this PR is accepted, then the templates/README.md file must be updated. (it is now updated)

Fixes #377

@kvid kvid linked an issue Jun 9, 2024 that may be closed by this pull request
@kvid kvid mentioned this pull request Jun 9, 2024
25 tasks
@formatc1702
Copy link
Collaborator

Please consider my comment on the original issue, but generally I am all in favor of making placeholder syntax consistent and therefore, of amending the Readme (please do) and merging this PR.

@kvid kvid force-pushed the release/v0.4.1-rc branch from 661f3cf to 2d38267 Compare June 12, 2024 01:08
kvid added a commit that referenced this pull request Jun 14, 2024
Fixes #377 (makes HTML output template placeholders more consistent)
kvid added a commit that referenced this pull request Jun 17, 2024
Fixes #377 (makes HTML output template placeholders more consistent)
Fixes #377 (makes HTML output template placeholders more consistent)
@formatc1702
Copy link
Collaborator

I see some recent force-pushes so I will wait for a final review request before merging :)

@kvid kvid requested a review from formatc1702 June 22, 2024 13:31
@formatc1702 formatc1702 merged commit 014c1f2 into release/v0.4.1-rc Jun 25, 2024
4 checks passed
@formatc1702 formatc1702 deleted the fix377 branch June 25, 2024 16:19
kvid added a commit that referenced this pull request Jul 5, 2024
Fixes #377 (makes HTML output template placeholders more consistent)
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.

[feature] Unfortunate HTML output template placeholder
2 participants