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

SLING-11706: Add basename provider to allow global basename setting i… #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

royteeuwen
Copy link

…nstead of only per component

@enapps-enorman
Copy link
Contributor

enapps-enorman commented Dec 6, 2022

Could this be solved with something like a looking for a well known request attribute name that specifies the default basename to use? Then you could use something like a servlet filter to set that request attribute for the pages where you can't or don't want to change the HTL files?

I'm thinking a request attribute may be more flexible than a component interface. For example, I would like to declare the default basename in the PostContruct init method of my page model so I don't have to keep repeating the basename on every i18n expression.

I've been using a technique (see example below) to shorten the repetitive basename on every string by declaring the basename as a variable at the top of the page and using that variable everywhere, but if I could declare the basename once in the page model's PostConstruct init method as a request attribute (or some other declarative way), it would simplify the HTL code as I could exclude the repetitive basename clauses everywhere.

For example, currently I have to do something like this:

<sly data-sly-set.rb="my.basename.here"></sly>

${"key1" @ i18n, basename=rb}
${"key2" @ i18n, basename=rb}

... or something equivalent to the the JSTL fmt:setBundle and fmt:bundle tags in HTL would make it easier.

@royteeuwen
Copy link
Author

@enapps-enorman nothing holding you back to do it that way, thats the reason of the provider interface. If you make a RequestAttributeBasenameProvider implementation and then set that attribute in the PostConstruct of the Page object then you have exactly what you want? I’m trying to keep it generic here so that anyway with their own wild idea can get it to work for their situation ;)

@enapps-enorman
Copy link
Contributor

One other concern I have is that perhaps I don't want others to replace the basename for the i18n strings of my pages. Someone could abuse what is proposed here to inject nonsense into the output. Is it really appropriate to do such things without coordination with the original authors?

@enapps-enorman nothing holding you back to do it that way, thats the reason of the provider interface. If you make a RequestAttributeBasenameProvider implementation and then set that attribute in the PostConstruct of the Page object then you have exactly what you want? I’m trying to keep it generic here so that anyway with their own wild idea can get it to work for their situation ;)

Perhaps that could work for the first part of what I was discussing, but it may not be quite as compatible with a solution of declaring the basename within the HTL for the page or a block.

In other words, if the HTL syntax was extended (using a JSTL fmt style approach) with a way to declare the basename for the page (or a block within the page), then juggling a well known request attribute could be a way to implement switching between the different basenames.

For example, I was thinking of a new syntax that looked something like this:

<!--/* before basename declared in the HTL so resolves to no basename 
         (or whatever was previously declared in the well-known request attribute via
          the page model, some servlet filter or otherwise)  */-->
${"key1.from.not.locally.declared.basename.bundle" @ i18n}

<!--/* declare basename for the whole page */-->
<sly data-sly-i18n.setBundle="my.pagewide.basename.here"></sly>

<!--/* basename resolves to the pagewide basename */-->
${"key1.from.pagewide.bundle" @ i18n}

<!--/* declare basename for this block of elements */-->
<sly data-sly-i18n.bundle="my.block.basename.here">
        <!--/* basename resolves to the block basename */-->
	${"key2.from.block.bundle" @ i18n}
</sly>

<!--/* the block basename is now out of scope and we are back to the pagewide basename again */-->
${"key2.from.pagewide.bundle" @ i18n}

Does that make sense?

@royteeuwen
Copy link
Author

Your first part makes sense but on the other hand its not different from overlaying / using resource type inheritance. If I’d want to extend the system, this is an entrypoint.

Adding another sly tag is something I cant make decisions about 😄, the spec is defined by Adobe. No idea if / how that would work

@enapps-enorman
Copy link
Contributor

enapps-enorman commented Dec 7, 2022

Adding another sly tag is something I cant make decisions about

I haven't looked at it too closely either to see it if is possible to do such things as a sling specific extension to the specification. Technically, the "basename" support isn't in the specification either as that is a sling specific extension.

https://sling.apache.org/documentation/bundles/scripting/scripting-htl.html#extensions-of-the-htl-specification-1

Maybe some Adobe people can chime in here to provide some guidance or feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants