D7517: filemerge: byteify the open() mode

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  In D7517#110547 <https://phab.mercurial-scm.org/D7517#110547>, @dlax wrote:
  
  >> This is actually pycompat.open(), so it need bytes.
  >
  > I don't understand why this is needed. The default value for "mode" as bytes comes from a407f9009392 <https://phab.mercurial-scm.org/rHGa407f900939257680a59c5279e481cf92605b98d>, but I don't understand the rationale.
  
  The reason for explicitly marking the mode there was that `sysstr(mode)` was already in place.  So that didn't change anything- `mode` was already required to be bytes.
  
  > Shouldn't we instead change all calls to `pycompat.open()` to use a native str for `mode`? (and drop `sysstr(mode)` in `pycompat.open()`).
  
  I suspect the byte `mode` was done to be consistent with the file name being bytes?  I agree with the first point though- I’d much rather this be an explicit call to `pycompat.open()` so that it’s clear that it isn’t the builtin function.  I noticed that *attr() builtins are similarly replaced, but I didn’t look inside to see what they were about. (I’m assuming they’re also about bytes -> str.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7517/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7517

To: mharbison72, #hg-reviewers
Cc: dlax, mercurial-devel