Skip to content

Conversation

n-i-x
Copy link
Contributor

@n-i-x n-i-x commented May 4, 2020

Description

Adds the ability to add functions to templates. Currently supports modifying a variable to all uppercase or all lowercase.

Usage:

{{.ProviderName | ToLower}} {{.ProviderName | ToUpper}}

Motivation and Context

I designed a template for oauth2-proxy which dynamically displays the provider login button based on the provider name using standard external CSS libraries. The required class names are btn-{{lowercaseProvider}}. Rather than create new variables to pass in to the template, the better solution seemed to be to allow the template engine to use basic string functions like ToUpper and ToLower.

HTML templates using external libraries may need to use the provider name in a class. As classes generally cannot be renamed unless a pre-processor like SASS or LESS is used, modifying the variable in the template is the best option.

How Has This Been Tested?

A template has been created which requires lowercase class names for the provider to display the provider logo using font-awesome.

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.

Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I'm surprised these aren't included by default.

Would be good to have a test for this, to make sure it doesn't panic, would you be willing to try and write a test?

Will also need a changelog entry please

templates.go Outdated
"ToUpper": strings.ToUpper,
"ToLower": strings.ToLower,
}
logger.Printf("using custom template directory %q", dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log line is duplicated with line 15

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll take a stab at a test. Not sure how I missed that duplicate logging entry...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests added... PTAL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGELOG entry added

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry about the lint error.. I used gitpod and my editor settings weren't configured there to format on save

JoelSpeed
JoelSpeed previously approved these changes May 8, 2020
Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

steakunderscore
steakunderscore previously approved these changes May 9, 2020
Copy link
Contributor

@steakunderscore steakunderscore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@steakunderscore steakunderscore dismissed stale reviews from JoelSpeed and themself via 516a59d May 9, 2020 19:59
@steakunderscore steakunderscore merged commit be9eaae into oauth2-proxy:master May 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants