jonhadfield / python-hosts

a hosts file manager library written in python
MIT License
125 stars 33 forks source link

fix entry repr quote #45

Closed trim21 closed 1 year ago

jonhadfield commented 1 year ago

Thanks @trim21. Will fix up the broken tests and check asap.

trim21 commented 1 year ago

can you make a rnew release?

jonhadfield commented 1 year ago

Will try to tomorrow.

jonhadfield commented 1 year ago

Now published.
I had to remove repr formatting for path as it resulted in Windows paths being normalised, e.g. C:\first\next\hosts being returned as C:\first\next\hosts, that returns an error if you attempt to use the repr output.

trim21 commented 1 year ago

Now published. I had to remove repr formatting for path as it resulted in Windows paths being normalised, e.g. C:\first\next\hosts being returned as C:\first\next\hosts, that returns an error if you attempt to use the repr output.

What's the difference between two path in your example...

trim21 commented 1 year ago

Screenshot_2023-08-29-02-51-32-722_com brave browser-edit

Didn't get your point, it looks fine for string repr with backward slash

jonhadfield commented 1 year ago

Seems GitHub has removed the double backslashes, just like the repr format did.

trim21 commented 1 year ago

Now I'm more confused, looks like my pr doesn't change repr with any path?

jonhadfield commented 1 year ago

On Windows only.
Your PR is merged. I simply had to remove repr formatting for path.

jonhadfield commented 1 year ago

Screenshot_2023-08-29-02-51-32-722_com brave browser-edit

Didn't get your point, it looks fine for string repr with backward slash

Sorry, I didn't see the image as it doesn't show on the GitHub app on iOS.

When Hosts had a path that didn't use string literals, and so included double backslashes, the repr (in the formatting) would show with single backslashes. As the repr needs to be something you can execute to reproduce the object, having single backslashes means it couldn't be evaluated. I thought about having the repr return Host(path=r'{0!r}'... but it looked messy in contrast. I also looked at Path but that isn't in 2.7 and I'm trying to retain that compatibilty.

trim21 commented 1 year ago

Sorry, I didn't see the image as it doesn't show on the GitHub app on iOS.

When Hosts had a path that didn't use string literals, and so included double backslashes, the repr (in the formatting) would show with single backslashes. As the repr needs to be something you can execute to reproduce the object, having single backslashes means it couldn't be evaluated. I thought about having the repr return Host(path=r'{0!r}'... but it looked messy in contrast. I also looked at Path but that isn't in 2.7 and I'm trying to retain that compatibilty.

Current repr of path in Hosts is still not valid...

from python_hosts import Hosts

print(repr(Hosts()))
Hosts(path='c:\windows\system32\drivers\etc\hosts', entries=[...])

it's expected to be Hosts(path='c:\\windows\\system32\\drivers\\etc\\hosts', entries=[...], repr of single backslash should be double backslashes, otherwise it should have a r prefix.

https://github.com/jonhadfield/python-hosts/blob/04bf6cddf9fce40cf317c0d3773b9467ad37fb06/python_hosts/hosts.py#L163-L166

So current manually quoted is not valid, it should be return 'Hosts(path={0!r}, entries={1!r})'.format(self.path, self.entries)