grass.script: Pass encoding to Popen parameters

[albertoparadisllop]

#1521

Added comment on why encoding kwarg is popped in start_command, it's that the rest of the kwargs will be used to construct the call to Popen.

I simply didnt assign it to any variable. It could also have been done with the del keyword.

A different approach could have been to, instead of popping "encoding", do the following:

@@ -464,18 +464,12 @@ def start_command(
 
     :return: Popen object
     """
-    # If encoding is in kwargs, remove it,
-    # as we dont want it to be used on the
-    # call to make_command
-    if "encoding" in kwargs:
-        kwargs.pop("encoding")
-
     options = {}
     popts = {}
     for opt, val in kwargs.items():
         if opt in _popen_args:
             popts[opt] = val
-        else:
+        elif opt != "encoding":
             options[opt] = val
 
     args = make_command(prog, flags, overwrite, quiet, verbose, **options)

Done as part of Hacktoberfest

[wenzeslaus]

@albertoparadisllop removing from make_command parameters makes sense.

@petrasovaa Popen has an encoding parameter. Do we want to pass it to Popen? For that, it seems that adding it to _popen_args would be the right solution. ...unless we want to ignore it.

[albertoparadisllop]

I just thought about it, and it might make more sense to pass it to Popen parameters, as the encoding kwarg is used by other methods to decode the Popen object stdout and stderr (using _make_unicode).

In other words, if I understand correctly, the methods that use Popen object outputs expect them to be encoded using that specific encoding, so it should be passed to Popen.

I'll wait for confirmation before jumping to conclusions though.

[petrasovaa]

I just thought about it, and it might make more sense to pass it to Popen parameters, as the encoding kwarg is used by other methods to decode the Popen object stdout and stderr (using _make_unicode).

In other words, if I understand correctly, the methods that use Popen object outputs expect them to be encoded using that specific encoding, so it should be passed to Popen.

I'll wait for confirmation before jumping to conclusions though.

I agree. If the encoding is specified, stdin/out/err will be unicode, so _make_unicode won't do anything, but that's fine I guess.