Complete O_flags used for file open #1853
Conversation
Src/IronPython/Modules/_io.cs
Outdated
| @@ -54,6 +55,19 @@ public static partial class PythonIOModule { | |||
|
|
|||
| private static int O_EXCL => RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 0x400 : RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? 0x800 : 0x80; | |||
|
|
|||
| [PythonHidden(PlatformsAttribute.PlatformFamily.Windows)] | |||
There was a problem hiding this comment.
Having PythonHidden on a private seems kind of odd but I guess it doesn't hurt...
There was a problem hiding this comment.
Yes, it doesn't hurt, but the less verbosity the better. I assume the same applies to internal, though I see there are a few PythonHidden that are internal already in the codebase.
There was a problem hiding this comment.
Weird, I wonder why we'd have them on internal members. Maybe something that was public at some point?
There was a problem hiding this comment.
Look for example at ByteArray.UnsafeByteList. It was established 5 years ago as internal, but 4 years ago you added PythonHidden in #1000. Do you remember why?
There was a problem hiding this comment.
Can't say I remember, but looking at #693 I changed Bytes.GetUnsafeByteArray (which was public) to a public property and then in a follow up commit made it internal. I guess I forgot to remove the PythonHidden when I did this. ByteArray.UnsafeByteList is probably a copy/paste...
Tests/test_file.py
Outdated
| @@ -701,7 +701,7 @@ def test_errors(self): | |||
|
|
|||
| with self.assertRaises(OSError) as cm: | |||
| open('path_too_long' * 100) | |||
| self.assertEqual(cm.exception.errno, (36 if is_posix else 22) if is_netcoreapp and not is_posix or sys.version_info >= (3,6) else 2) | |||
| self.assertEqual(cm.exception.errno, (63 if is_osx else 36 if is_linux else 22) if is_netcoreapp and not is_posix or sys.version_info >= (3,6) else 2) | |||
There was a problem hiding this comment.
Not that I mind the explicit number check, but wondering if we should be using the errno module values instead to make things easier to decipher.
There was a problem hiding this comment.
Good point, assuming that errno is not buggy 😄
Includes part of the solution for #1225.