search

OctopusDeploy / Octostache

| Public | The variable substitution syntax for Octopus Deploy
Other
5 stars 24 forks source link

filters: add urlEncode function #29

Closed Pondidum closed 5 years ago

Pondidum commented 5 years ago

I've had a few use cases recently for needing to escape a variable for putting in a URL, so this is a little PR to add this feature, rather than me having to keep making duplicate versions of a variable, but URL encoded.

I couldn't see any docs to add examples for this (I was looking for the source of this page.

CLAassistant commented 5 years ago

CLA assistant check
All committers have signed the CLA.

Pondidum commented 5 years ago

Interesting! I had based my usage off of this StackOverflow answer: https://stackoverflow.com/a/7878050/1500 which implied that Uri.EscapeDataString was the right thing to use for this. Originally I was using WebUtility.UrlEncode (which produces the same results as Uri.EscapeDataString in my testing), but as the Octostash library targets 4.0, WebUtility can't be used (available 4.5+).

I can make some changes (and a few more tests I think) tomorrow when I get back home. I think the idea of renaming and including both variants might be best.

Thanks for the feedback.

Pondidum commented 5 years ago

Nice to see that all the methods provide slightly different outputs!

> using System.Net;
> var input = "A b:c+d/e";

> Uri.EscapeDataString(input) // "A%20b%3Ac%2Bd%2Fe"
> Uri.EscapeUriString(input)  // "A%20b:c+d/e"
> WebUtility.UrlEncode(input) // "A+b%3Ac%2Bd%2Fe"

I've replaced the urlencode function I added with the two as suggested, but named them UriDataEscape and UriEscape to fit with the other function's naming style.

Also not sure if you'd rather I squashed these into one commit, or are fine with the two separate commits - I don't mind either way!

matt-richardson commented 5 years ago

Thanks @Pondidum! Sorry it took us so long to merge it.