Add a check for bad filenames in the directory containing the user's code

[schmave]

This is a draft of a solution that could work. What do you think? Here's a sample error message:

(venv) Evans-MacBook-Pro:test-cmu evan$ python test.py 

******************************************************************************
* Warning: The following files in /Users/evan/test-cmu
* may prevent your program from running correctly. Please rename these files.
*
* select.py
******************************************************************************

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/subprocess.py", line 69, in <module>
    import msvcrt
ModuleNotFoundError: No module named 'msvcrt'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/evan/test-cmu/test.py", line 1, in <module>
    from cmu_graphics import *
  File "/Users/evan/test-cmu/venv/lib/python3.10/site-packages/cmu_graphics/__init__.py", line 160, in <module>
    from .libs import loader_util
  File "/Users/evan/test-cmu/venv/lib/python3.10/site-packages/cmu_graphics/libs/loader_util.py", line 2, in <module>
    import platform
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/platform.py", line 119, in <module>
    import subprocess
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/subprocess.py", line 75, in <module>
    import select
  File "/Users/evan/test-cmu/select.py", line 6, in <module>
    cmu_graphics.run()
NameError: name 'cmu_graphics' is not defined

[austin-schick]

Looks pretty good to me! What do you think about exiting immediately when we print the warning? Some arguments for that behavior:

• For some of these modules, we're pretty confident that the bad filename is going to cause a crash anyway
• Our warning could become lost above the stack trace when the program exits
• It might be nicer to fail immediately instead of waiting and failing in a confusing way down the line

I don't have a super strong opinion either way, but I do think it's worth thinking about.

[dkosbie]

How about both, main_directory and cwd? Also, I think we should hide the rest of the error message (the stack trace), so they only see the part telling them to rename or remove the offending file (and I think we should say that, "rename or remove"). Thx.

On Thu, Apr 13, 2023 at 1:33 PM Austin Schick @.***> wrote:

@.**** commented on this pull request.

In cmu_graphics/init.py https://github.com/cmu-cs-academy/desktop-cmu-graphics/pull/41#discussion_r1165837506 :

@@ -1,4 +1,56 @@ +import main +import os + +main_directory = os.path.dirname(main.file) +main_sibling_filenames = set(os.listdir())

I think we probably want to use main_directory in this call to os.listdir(). The docs say: "The first entry in the module search path is the directory that contains the input script". So we do want main_directory, not the current working directory.

— Reply to this email directly, view it on GitHub https://github.com/cmu-cs-academy/desktop-cmu-graphics/pull/41#pullrequestreview-1383905369, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3PPRGKU4LFKUNGSYYTNQLXBA2HVANCNFSM6AAAAAAW5L2OQE . You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

[dkosbie]

Right, what Austin just said. Clear early fail.

On Thu, Apr 13, 2023 at 1:42 PM Austin Schick @.***> wrote:

Looks pretty good to me! What do you think about exiting immediately when we print the warning? Some arguments for that behavior:

• For some of these modules, we're pretty confident that the bad filename is going to cause a crash anyway
• Our warning could become lost above the stack trace when the program exits
• It might be nicer to fail immediately instead of waiting and failing in a confusing way down the line

I don't have a super strong opinion either way, but I do think it's worth thinking about.

— Reply to this email directly, view it on GitHub https://github.com/cmu-cs-academy/desktop-cmu-graphics/pull/41#issuecomment-1507369551, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3PPRDAWDB7Y6G6AW72UQDXBA3IJANCNFSM6AAAAAAW5L2OQE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[austin-schick]

@palascat100 and @josht04 I think this is ready for you to test. I'm leaving out Alex because he's on Linux, which Desktop CMU Graphics doesn't support.

[austin-schick]

Thanks!