Patch Error: UnicodeDecodeError

[psadi]

Im trying to patch the font and facing the below error, any advice ?

[psadi]

Not sure what kind of bug this is, but when re-ran with verbose the script exits, correctly and im able to get the patched font

https://github.com/daylinmorgan/monolisa-nerdfont-patch/assets/22428691/0907ce92-f0c7-45e6-871f-114f972faf8c

[daylinmorgan]

@psadi could you verify if the recent commit runs successfully on your machine without --verbose?

Also, just to make sure you know, with the current command you are only adding three symbol sets, see font-patcher --help for a list of the symbols and their corresponding flags or use -c/--complete to add all of them.

[psadi]

The issue still remains,

Without Verbose & With -c

With Verbose & With -c:

[daylinmorgan]

Yay encoding errors.... docker or your system is returning text with a different encoding. I think I can catch this at runtime.

What is the output of the following command:

python -c 'import locale; print(locale.getpreferredencoding())'

[psadi]

Hi @daylinmorgan ,

I just tested the patched font locally with encoding=None, and it seems to be working correctly!

However, I'm not sure if there might be any unintended side effects from this change

Would it be okay to submit a pull request with this fix, or should we consider adding it directly?

[psadi]

Yay encoding errors.... docker or your system is returning text with a different encoding. I think I can catch this at runtime.

What is the output of the following command:

python -c 'import locale; print(locale.getpreferredencoding())'

Please find output

[daylinmorgan]

Removing the encoding all together will be an issue if there is an error in the subprocess because then the stdout/stderr is returned to the user: https://github.com/daylinmorgan/monolisa-nerdfont-patch/blob/6f5ec7e73e781a11cd37dceb698a9ad3bd8777ee/patch-monolisa#L97-L105

I don't know why the output of docker run seems to not be UTF-8

Could you try running it after setting the encoding to "ISO-8859-1"?

I noticed your python was installed by homebrew are you using OS X?

Really the simplest solution for this script is probably to just ignore encoding errors, by setting errors='replace' in the subprocess.run

[psadi]

Correct, I'm currently on OSX. Wouldn't it be better to enclose the subprocess block in try..except and catch explicit errors.

I was thinking something like this so None would be in use for unsupported encodings and still catch errors for a graceful exit.

import locale

try:
  p = subprocess.run(
      command,
      stdout=None if verbose else subprocess.PIPE,
      stderr=None if verbose else subprocess.STDOUT,
      encoding = "utf-8" if locale.getpreferredencoding().lower() == 'utf-8' else None
  )
except (CalledProcessError, UnicodeDecodeError, ValueError) as err:
  # do something

[psadi]

Removing the encoding all together will be an issue if there is an error in the subprocess because then the stdout/stderr is returned to the user:

https://github.com/daylinmorgan/monolisa-nerdfont-patch/blob/6f5ec7e73e781a11cd37dceb698a9ad3bd8777ee/patch-monolisa#L97-L105

I don't know why the output of docker run seems to not be UTF-8

Could you try running it after setting the encoding to "ISO-8859-1"?

I noticed your python was installed by homebrew are you using OS X?

Really the simplest solution for this script is probably to just ignore encoding errors, by setting errors='replace' in the subprocess.run

A noteworthy point: I faced the same error using make patch as well

[daylinmorgan]

Wouldn't it be better to enclose the subprocess block in try..except and catch explicit errors.

Not really because we don't actually need the output for anything so might as well lazily decode it and move on. Also subproccess.run doesn't raise a CalledProccessError without check = True.

Also, as we've learned this line encoding = "utf-8" if locale.getpreferredencoding().lower() == 'utf-8' else None on your machine would set the encoding to utf-8 which it's not.

A noteworthy point: I faced the same error using make patch as well

Ya make patch doesn't do anything special.

[psadi]

Agreed, so we could just catch UnicodeDecodeError and re-run as None. I don't seem to find docker as the issue here as the same behaviour is seen with make patch without docker

[daylinmorgan]

We don't want to run the subprocess twice because the error has nothing to do with it. And this doesn't solve the problem I mentioned here that we still want to decode the result to print in the case of errors

You have fontforge installed? Is there a reason you are using the docker mode then?

What is the output with make patch or patch-monolisa -f Monolisa -c if you are still getting encoding errors then I think this has something to with your environment of locale settings.