openalm / Extension-UtilitiesPack

Release Management utility tasks
Other
34 stars 38 forks source link

Set token to empty value #24

Closed pascalberger closed 7 years ago

pascalberger commented 8 years ago

Currently it seems to not be possible to set the value of a token in the tokenizer task to an empty value. If I have a file containing the token __foo__ and a variable with no value set, the token is not replaced (Test-Path env:$matchedItem returns false).

It would be nice if the Tokenizer would also support to replace tokens with empty strings.

pascalberger commented 8 years ago

@anangaur Any idea about this?

harshil93 commented 8 years ago

@pascalberger Yes this is a bug. I will try to fix it asap.

harshil93 commented 8 years ago

I tried setting an environment variable in powershell with empty value and it doesn't set that variable. [Environment]::SetEnvironmentVariable("TestVariable", "", "User") (This doesn't set the env variable)

I think an env variable with empty value means you are unsetting that environment variable and thus it is no longer passes the test-path env:$matchedItem check.

pascalberger commented 8 years ago

@harshil93 Yes this was also my observation. Any approach how this should be solved? Would setting an empty value in case neither environment variable was set nor value was found in configuration be an option?

pascalberger commented 8 years ago

@harshil93 @anangaur Any opinion about this?

MarcelMichau commented 8 years ago

I agree with @pascalberger. If an environment variable is unset (i.e. not found in configuration) by means of assigning it to an empty string, then the replaced variable should default to an empty string. i.e in the very last else block of the tokenise.ps1 script: Write-Host "No value found for token '$match' - setting to empty string" $variableValue = [string]::Empty works fine for me at the moment.

pascalberger commented 8 years ago

I've created a PR for this: #48. @harshil93 @anangaur Any chance to get this merged and released?

harshil93 commented 8 years ago

@pascalberger - One issue I can see with setting a not defined variable to an empty is breaking existing behavior if someone later in their tasks is doing something with unassigned variables.

Given this, I think an optional checkbox option telling user to replace undefined with empty ones would be better rather than just changing the behavior.

pascalberger commented 8 years ago

@harshil93 Thanks for the feedback. I'll update the PR with a user checkbox. But propably will wait until #50 is merged and rebase on this.

harshil93 commented 7 years ago

@pascalberger The PR is merged. If you want to contribute to this. Go ahead.

pascalberger commented 7 years ago

@harshil93 Since you have closed this issue do you already merge the PR (#48)? Otherwise the issue is not fixed...

harshil93 commented 7 years ago

@pascalberger I will try to get it merged by today. Reopening the issue till it get merged.