lipkau / PsIni

Work with INI files in PowerShell using hashtables
http://lipkau.github.io/PsIni
MIT License
152 stars 52 forks source link

Quotes stripped and trimming #50

Open majkinetor opened 5 years ago

majkinetor commented 5 years ago

I used this code and compared the results:

Get-IniContent $iniPath\wincmd.ini | Out-IniFile $iniPath\wincmd2.ini

The difference, appart from white lines, were in the lines which had values that are within quotes:

key="some value"

This will become after output:

key=some value

This is a problem:

param7=""%n"" ==> param7="%n"

because standard ini reader will get n instead "n".

I noticed this because custom application I added in Total Commander lost quotes around parameter and consequetlly didn't work any more as expected.

lipkau commented 5 years ago

https://github.com/lipkau/PsIni/blob/479a399f95d2630edc62f16cf90014242acfd5c4/PSIni/Functions/Get-IniContent.ps1#L92

Indeed. This was changed in 50f6878c8d9c330831b711cf317cdbe85d65a25c original regex was (.+?)\s*=\s*(.*)

majkinetor commented 5 years ago

It still doesn't work with latest version

vikingtiger commented 5 years ago

I saw the "help wanted" label and thought I'd help, but before I attempt a fix, I'd like to clarify something. What was the reason for excluding ['`"] from the value in Get-IniContent in commit 50f6878? If we were to change the regex to include the quotes in the value group (e.g. by changing the character class in group 2 to ['`] or simply `), do you anticipate any issues with any implementations which do not tolerate quote-encapsulated values?

lipkau commented 5 years ago

I do believe it will be a breaking change, yes. Therefore I will increate the major version (and did not decide if I wait for #49 or not).

I am also not sure if it should only be rolled back to (.+?)\s*=\s*(.*) or to \s*(.+?)\s*=\s*(.*)\s* to trim the whitespaces. maybe go first the first one, and leave the decision on the trimming to be part of #49

majkinetor commented 5 years ago

to trim the whitespaces.

Why would you trim whitespace, since it constitutes a value too ?

Example: https://www.mjtnet.com/forum/viewtopic.php?t=9339 Wiki: https://en.wikipedia.org/wiki/INI_file#Whitespace

Again, like with #49, this is all cool as an explicit configurable option (either function parameter or module config) but as a default will break stuff.


Related reference about quotes: https://en.wikipedia.org/wiki/INI_file#Quoted_values

lipkau commented 5 years ago

Why would you trim whitespace, since it constitutes a value too ? Not sure that even

Example: https://www.mjtnet.com/forum/viewtopic.php?t=9339 Wiki: https://en.wikipedia.org/wiki/INI_file#Whitespace

I mean the leading and trailing whitespaces. (" text ").trim() <--- like that

lipkau commented 5 years ago

After reading your reference

Related reference about quoes: https://en.wikipedia.org/wiki/INI_file#Quoted_values

I think the behavior of the module as-is, is well aligned with that description. Here are some examples: https://regex101.com/r/3ssNoB/2 main focus on matches 6, 7 and 8

majkinetor commented 5 years ago

I mean the leading and trailing whitespaces. (" text ").trim() <--- like that

Yeah, I mean about it too, that is invalid. I am honestly surprised to see that some parsers collapsed multiple internal spaces...

So, in your regex example where _ is space:

__key__ =__value__

then $ini.key should be __value__, not value. I.E. imagine I need to specify field separator to space. The following two should be identical:

key=_
key="_"

I understand that some folks would like automatic parsing rather then $ini.Key.Trim() all the time, hence the option.

vikingtiger commented 5 years ago

Interesting discussions. The INI file format is indeed quite loosely defined, and different implementations enforce different rules. De-quoting values does, in my opinion, make sense in certain situations, as does trimming leading and trailing whitespaces. Configurable behaviour might be the only compromise that can please everyone, but it adds complexity.

To add to this discussion, might I suggest a "Raw" parameter (or something similar) to Get-IniContent? As the name suggests, this could ensure that values are preserved "as is", so that if you write it back to an INI file using Out-IniFile, as @majkinetor demonstrated in his original post, the values are unaltered.

It is my impression that this and related issues need more discussion, and I do not think any pull requests from me would be useful at this point.

lipkau commented 4 years ago

about the

__key__ =__value__

I update the regex snip with more examples and a little tweak about the trailing whitespace and the whitespaces around the =: https://regex101.com/r/3ssNoB/3 what is the expected behavior regarding whitespaces around the =? key_=_value

majkinetor commented 4 years ago

what is the expected behavior regarding whitespaces around the =? key_=_value

Left of = is ignored. Right of = is part of the value unless some "trim option" is present.

lipkau commented 4 years ago

ok. I am fine with that... user is responsible for trimming updated https://regex101.com/r/3ssNoB/4

now back to the quotes: you are saying these two are not the same for the Windows API, correct?

key="value"
key=value
majkinetor commented 4 years ago

AFAIK they should be the same. However, you can't remove quote when saving back as:

key=""value"" -- save -->key="value" -- save --> key = value

So, point is, if I give you the value entirelly in ", it should stay that way.

I guess at this point what is needed here and for the other question of trimming is adding pester tests for windows compatibility.

lipkau commented 4 years ago

I just tested with your script using your custom ini script

image

with the following ini file:

[test]
key1=foo
key2 = "bar"
key3 = ""baz""

So I guess we are close to the original regex ((.+?)\s*=\s*(.*)).

So to anyone willing to submit a PR on this, here is the regex I would recommend: ^\s*(.+?)\s*=(.*)$ https://regex101.com/r/3ssNoB/5 Please don't forget to add unit tests 🤣

majkinetor commented 4 years ago

What about getting and setting key value ?

lipkau commented 4 years ago

I don't understand the question. What about it?

majkinetor commented 4 years ago

Here is what I ment, I tested with this script:

TEST.INI

[test]
key1=foo
key2 = "bar"
key3 = ""baz""

test.ps1

. ./ini.ps1  #https://github.com/majkinetor/au-packages/blob/master/tcp/tcps/tools/Ini.ps1

$iniPath = Resolve-Path 'TEST.INI'

$s = Get-IniSection $iniPath test 
$s = $s -split "`n" -join [char]0  #Required by ProfileAPI, key/values are separated by NULL
Set-IniSection $iniPath test_section $s

"-"*10

$v1 = Get-IniKey $iniPath test key1
$v2 = Get-IniKey $iniPath test key2
$v3 = Get-IniKey $iniPath test key3

$v1, $v2, $v3

Set-IniKey $iniPath test_key key1 $v1
Set-IniKey $iniPath test_key key2 $v2
Set-IniKey $iniPath test_key key3 $v3

if ($v3.StartsWith('"')) { $v3 = '"{0}"' -f $v3 }
Set-IniKey $iniPath test_key2 key3 $v3

[test]
key1=foo
key2 = "bar"
key3 = ""baz""
[test_section]
key1=foo
key2="bar"
key3=""baz""
[test_key]
key1=foo
key2=bar
key3="baz"
[test_key2]
key3=""baz""

It shows dobule standard and that Profile API will:

majkinetor commented 4 years ago

TEST.ini

[test]
key1=foo
key2 = "bar"
key3 = ""baz""
key4 =       meh      
key5 ="       meh      "

test.ps1

. ./ini.ps1 #https://github.com/majkinetor/au-packages/blob/master/tcp/tcps/tools/Ini.ps1

$iniPath = Resolve-Path 'TEST.INI'

$s = Get-IniSection $iniPath test 
$s = $s -split "`n" -join [char]0
Set-IniSection $iniPath test_section $s

"-"*10

1..5 | % { 
    $v = Get-IniKey $iniPath test key$_ 
    Set-IniKey $iniPath test_key key$_ $v
}

$v = Get-IniKey $iniPath test_key key5
Set-IniKey $iniPath test_key key52 $v
[test]
key1=foo
key2 = "bar"
key3 = ""baz""
key4 =       meh      
key5 ="       meh      "

[test_section]
key1=foo
key2="bar"
key3=""baz""
key4=meh
key5="       meh      "
[test_key]
key1=foo
key2=bar
key3="baz"
key4=meh
key5=       meh      
key52=meh

Now this is awesome inconsistent behavior

Jonny-vb commented 2 years ago

This is a massive pain for me. I was hoping to use PsIni to edit a large existing ini file, however sections I am not changing are getting changed anyway because quotation marks are being stripped!

YuS-2 commented 1 year ago

Compromise: https://regex101.com/r/3ssNoB/7

...
                $name, $value = $matches[1, 2]
...