Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(90918)

#21719: Returning Windows file attribute information via os.stat()

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 2 months ago by benhoyt
Modified:
5 years, 2 months ago
Reviewers:
zachary.ware, ethan, victor.stinner, martin
CC:
loewis, haypo, benhoyt, stoneleaf, Jim.J.Jewett, Zach Ware
Visibility:
Public.

Patch Set 1 #

Total comments: 18

Patch Set 2 #

Total comments: 2

Patch Set 3 #

Patch Set 4 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/os.rst View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download
Doc/library/stat.rst View 1 2 3 2 chunks +27 lines, -1 line 0 comments Download
Lib/stat.py View 1 2 3 1 chunk +23 lines, -0 lines 0 comments Download
Lib/test/test_os.py View 1 2 3 1 chunk +19 lines, -0 lines 0 comments Download
Lib/test/test_stat.py View 1 2 3 3 chunks +29 lines, -0 lines 0 comments Download
Modules/posixmodule.c View 1 2 3 6 chunks +16 lines, -0 lines 0 comments Download
Modules/_stat.c View 1 2 3 3 chunks +34 lines, -0 lines 0 comments Download

Messages

Total messages: 6
Zach Ware
http://bugs.python.org/review/21719/diff/12149/Modules/_stat.c File Modules/_stat.c (right): http://bugs.python.org/review/21719/diff/12149/Modules/_stat.c#newcode562 Modules/_stat.c:562: #ifdef MS_WINDOWS Even though these constants are really only ...
5 years, 2 months ago #1
stoneleaf
http://bugs.python.org/review/21719/diff/12149/Doc/library/stat.rst File Doc/library/stat.rst (right): http://bugs.python.org/review/21719/diff/12149/Doc/library/stat.rst#newcode129 Doc/library/stat.rst:129: An additional utility function is provided to covert a ...
5 years, 2 months ago #2
victor.stinner_gmail.com
You may also add hardcoded constants to the stat.py module, as done for UNIX constants, ...
5 years, 2 months ago #3
victor.stinner_gmail.com
http://bugs.python.org/review/21719/diff/12149/Doc/library/stat.rst File Doc/library/stat.rst (right): http://bugs.python.org/review/21719/diff/12149/Doc/library/stat.rst#newcode425 Doc/library/stat.rst:425: On 2014/06/13 15:49:58, stoneleaf wrote: > Instead of spelling ...
5 years, 2 months ago #4
loewis
http://bugs.python.org/review/21719/diff/12149/Doc/library/stat.rst File Doc/library/stat.rst (right): http://bugs.python.org/review/21719/diff/12149/Doc/library/stat.rst#newcode406 Doc/library/stat.rst:406: constants. On 2014/06/13 16:00:20, haypo wrote: > It would ...
5 years, 2 months ago #5
Zach Ware
5 years, 2 months ago #6
http://bugs.python.org/review/21719/diff/12149/Modules/_stat.c
File Modules/_stat.c (right):

http://bugs.python.org/review/21719/diff/12149/Modules/_stat.c#newcode562
Modules/_stat.c:562: #ifdef MS_WINDOWS
On 2014/06/13 16:00:20, haypo wrote:
> On 2014/06/13 15:49:28, Zach Ware wrote:
> > Even though these constants are really only useful on Windows, I wonder if
we
> > shouldn't define them on all platforms anyway since all other stat constants
> are
> > at least defined (if only to 0).
> > 
> > Either way, these should probably be defined in stat.py as well (sorry; I
> should
> > have said 'as well as' instead of 'instead of' when I mentioned _stat.c in
the
> > email thread).
> 
> The purpose of the _stat module is to only provide data from the OS, to not
> hardcode constants. So -1 to provide these constants on all platforms.

This is just the opposite of the situation for the rest of the constants in this
module, most of which are not defined outside of this module on Windows.  I'm
not terribly bothered either way, though.

http://bugs.python.org/review/21719/diff/12154/Doc/library/stat.rst
File Doc/library/stat.rst (right):

http://bugs.python.org/review/21719/diff/12154/Doc/library/stat.rst#newcode405
Doc/library/stat.rst:405: See the `Windows API documentation
<http://msdn.microsoft.com/en-us/library/windows/desktop/gg258117.aspx>`_
Better two short lines than one long one :)

http://bugs.python.org/review/21719/diff/12154/Lib/test/test_os.py
File Lib/test/test_os.py (right):

http://bugs.python.org/review/21719/diff/12154/Lib/test/test_os.py#newcode544
Lib/test/test_os.py:544: self.assertTrue(result.st_file_attributes & 0x10 == 0)
I agree with Victor, better to use the shiny new stat module constant.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+