This is an archive of the discontinued Mercurial Phabricator instance.

hgext/logtoprocess.py
54	seems like this should be `pycompat.iswindows` (which probably doesn't exist yet, but should) because comparisons to things that should be constants scattered throughout the code scare me.

This revision now requires changes to proceed.Oct 12 2017, 10:53 AM

quark added inline comments.Oct 12 2017, 10:57 AM

hgext/logtoprocess.py
54	This check is common in the code base: mercurial/scmutil.py 44:if pycompat.osname == 'nt': 294: abort = pycompat.osname == 'nt' or lval == 'abort' mercurial/i18n.py 32:if (pycompat.osname == 'nt' mercurial/rcutil.py 18:if pycompat.osname == 'nt': mercurial/subrepo.py 1350: elif pycompat.osname == 'nt': mercurial/color.py 213: if pycompat.osname == 'nt': 382:if pycompat.osname == 'nt': mercurial/vfs.py 546: defaultenabled = pycompat.osname == 'nt' mercurial/util.py 95:if pycompat.osname == 'nt': 1351:if pycompat.osname == 'nt': mercurial/ui.py 1044: if pycompat.osname == 'nt' and not shell: mercurial/sslutil.py 480: pycompat.osname == 'nt'): 720: if pycompat.osname == 'nt': hgext/win32mbcs.py 193: if pycompat.osname == 'nt': hgext/logtoprocess.py 55: if pycompat.osname == 'nt': hgext/schemes.py 119: if (pycompat.osname == 'nt' and len(scheme) == 1 and scheme.isalpha() mercurial/hgweb/server.py 269: if pycompat.osname == 'nt': hgext/convert/subversion.py 106: if pycompat.osname == 'nt': 257: if (pycompat.osname == 'nt' and path[:1] == '/' hgext/largefiles/lfutil.py 78: if pycompat.osname == 'nt':

hgext/logtoprocess.py

This check is common in the code base:

mercurial/scmutil.py
44:if pycompat.osname == 'nt':
294:    abort = pycompat.osname == 'nt' or lval == 'abort'

mercurial/i18n.py
32:if (pycompat.osname == 'nt'

mercurial/rcutil.py
18:if pycompat.osname == 'nt':

mercurial/subrepo.py
1350:            elif pycompat.osname == 'nt':

mercurial/color.py
213:    if pycompat.osname == 'nt':
382:if pycompat.osname == 'nt':

mercurial/vfs.py
546:        defaultenabled = pycompat.osname == 'nt'

mercurial/util.py
95:if pycompat.osname == 'nt':
1351:if pycompat.osname == 'nt':

mercurial/ui.py
1044:        if pycompat.osname == 'nt' and not shell:

mercurial/sslutil.py
480:                pycompat.osname == 'nt'):
720:    if pycompat.osname == 'nt':

hgext/win32mbcs.py
193:        if pycompat.osname == 'nt':

hgext/logtoprocess.py
55:    if pycompat.osname == 'nt':

hgext/schemes.py
119:        if (pycompat.osname == 'nt' and len(scheme) == 1 and scheme.isalpha()

mercurial/hgweb/server.py
269:    if pycompat.osname == 'nt':

hgext/convert/subversion.py
106:        if pycompat.osname == 'nt':
257:            if (pycompat.osname == 'nt' and path[:1] == '/'

hgext/largefiles/lfutil.py
78:    if pycompat.osname == 'nt':

quark mentioned this in D1033: pycompat: define operating system constants.Oct 12 2017, 12:37 PM

It's changed by D1034.

quark mentioned this in rHGc0a6c19690ff: pycompat: define operating system constants.Oct 13 2017, 10:05 AM

Closed by commit rHG68ed3b4f86ef: logtoprocess: do not use platform.system() (authored by quark). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			hgext/logtoprocess.py (8 lines)

Status	Author	Revision
Closed	quark	D1037 check-code: suggest pycompat.is(posix\|windows\|darwin)
Closed	quark	D1035 codemod: use pycompat.isdarwin
Closed	quark	D1036 codemod: use pycompat.isposix
Closed	quark	D1034 codemod: use pycompat.iswindows
Closed	quark	D1033 pycompat: define operating system constants
Closed	quark	D1024 hgweb: do not import uuid immediately to avoid its side effect
Closed	quark	D1021 check-code: forbid platform.system()
Closed	quark	D1020 largefiles: do not use platform.system()
Closed	quark	D1019 logtoprocess: do not use platform.system()
Closed	quark	D1018 selectors2: do not use platform.system()

Diff 2678

hgext/logtoprocess.py

	not ensure that they exit cleanly.			not ensure that they exit cleanly.

	"""			"""

	from __future__ import absolute_import			from __future__ import absolute_import

	import itertools			import itertools
	import os			import os
	import platform
	import subprocess			import subprocess
	import sys			import sys

	from mercurial import encoding			from mercurial import (
				encoding,
				pycompat,
				)

	# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for			# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for
	# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should			# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
	# be specifying the version(s) of Mercurial they are tested with, or			# be specifying the version(s) of Mercurial they are tested with, or
	# leave the attribute unspecified.			# leave the attribute unspecified.
	testedwith = 'ships-with-hg-core'			testedwith = 'ships-with-hg-core'

	def uisetup(ui):			def uisetup(ui):
	if platform.system() == 'Windows':			if pycompat.osname == 'nt':
				ryanmceUnsubmitted Not Done seems like this should be `pycompat.iswindows` (which probably doesn't exist yet, but should) because comparisons to things that should be constants scattered throughout the code scare me. ryanmce: seems like this should be `pycompat.iswindows` (which probably doesn't exist yet, but should)…
				quarkAuthorUnsubmitted Not Done This check is common in the code base: mercurial/scmutil.py 44:if pycompat.osname == 'nt': 294: abort = pycompat.osname == 'nt' or lval == 'abort' mercurial/i18n.py 32:if (pycompat.osname == 'nt' mercurial/rcutil.py 18:if pycompat.osname == 'nt': mercurial/subrepo.py 1350: elif pycompat.osname == 'nt': mercurial/color.py 213: if pycompat.osname == 'nt': 382:if pycompat.osname == 'nt': mercurial/vfs.py 546: defaultenabled = pycompat.osname == 'nt' mercurial/util.py 95:if pycompat.osname == 'nt': 1351:if pycompat.osname == 'nt': mercurial/ui.py 1044: if pycompat.osname == 'nt' and not shell: mercurial/sslutil.py 480: pycompat.osname == 'nt'): 720: if pycompat.osname == 'nt': hgext/win32mbcs.py 193: if pycompat.osname == 'nt': hgext/logtoprocess.py 55: if pycompat.osname == 'nt': hgext/schemes.py 119: if (pycompat.osname == 'nt' and len(scheme) == 1 and scheme.isalpha() mercurial/hgweb/server.py 269: if pycompat.osname == 'nt': hgext/convert/subversion.py 106: if pycompat.osname == 'nt': 257: if (pycompat.osname == 'nt' and path[:1] == '/' hgext/largefiles/lfutil.py 78: if pycompat.osname == 'nt': quark: This check is common in the code base: ``` mercurial/scmutil.py 44:if pycompat.osname == 'nt'…
	# no fork on Windows, but we can create a detached process			# no fork on Windows, but we can create a detached process
	# https://msdn.microsoft.com/en-us/library/windows/desktop/ms684863.aspx			# https://msdn.microsoft.com/en-us/library/windows/desktop/ms684863.aspx
	# No stdlib constant exists for this value			# No stdlib constant exists for this value
	DETACHED_PROCESS = 0x00000008			DETACHED_PROCESS = 0x00000008
	_creationflags = DETACHED_PROCESS \| subprocess.CREATE_NEW_PROCESS_GROUP			_creationflags = DETACHED_PROCESS \| subprocess.CREATE_NEW_PROCESS_GROUP

	def runshellcommand(script, env):			def runshellcommand(script, env):
	# we can't use close_fds and redirect stdin. I'm not sure that we			# we can't use close_fds and redirect stdin. I'm not sure that we

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	2606		Oct 11 2017, 8:57 PM	★	★
Diff 2	2678	rHG68ed3b4f86ef506a0937bdf9e783146c37b09736	Oct 11 2017, 8:38 PM	★	★

This is an archive of the discontinued Mercurial Phabricator instance.

logtoprocess: do not use platform.system()ClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 2678

hgext/logtoprocess.py

logtoprocess: do not use platform.system()
ClosedPublic

Revision Contents
Changeset List