Conversation
Merges main into help50 branch
This comment was marked as off-topic.
This comment was marked as off-topic.
drqsatoshi
left a comment
There was a problem hiding this comment.
this PR #210 is still waiting for approval by an approver who has write access, btw.
|
All checks passed, still need 1 approving review by you @rongxin-liu . Looks like all checks passed. |
Thanks for the reminder. How did you perform the test? |
drqsatoshi
left a comment
There was a problem hiding this comment.
Major Changes
-
Help50 Installation Method
Removed: Python-based help50 package (line 262 in Dockerfile)
Added: Shell-based implementation with new dependencies:
colorized-logs
expect
file
fzf -
New Shell-Based Help50 System
The PR introduces a complete shell-based help system with several new files:
/opt/cs50/bin/help50: Main help50 binary
/etc/profile.d/help50.sh: Core help50 shell functionality (~128 lines)
/opt/cs50/lib/cli: Shared utility functions for alerts, ANSI formatting, file finding, and user prompts
Helper scripts in /opt/cs50/lib/help50/:
bash: Catches common bash errors (command not found, permission issues, syntax errors)
cd: Helps with directory navigation errors
clang: Catches compilation errors (missing main function)
make: Catches make-related errors
python: Catches Python import errors and file not found issues
-
Automatic Help50 Integration
etc/profile.d/cli.sh: Now automatically starts help50 if enabled (lines 66-68)
etc/profile.d/help50.sh:
Sets up PROMPT_COMMAND=_help50 to run after every command
Captures command output via typescript
Analyzes exit codes and output to provide contextual help
Creates helpful aliases to prevent accidental yes/no responses -
Improved User Check
Changed from whoami to id -u for checking if user is root (more reliable)
-
Environment Variable
Added WORKDIR=/mnt environment variable to the Makefile's docker run command
Key Features of the New Help50
Proactive Error Detection: Runs after every command via PROMPT_COMMAND
Context-Aware: Captures command output and analyzes it with specialized helpers
Common Error Patterns: Catches mistakes like:
Running Python files without python command
Typos (e.g., 1s instead of ls)
Wrong slashes (.\foo instead of ./foo)
Missing file compilation steps
Module shadowing in Python
Missing main function in C programs
File/directory confusion
Smart Suggestions: Uses file system search to suggest corrections when files exist in subdirectories
Potential Concerns
Performance: Running help50 after every command could add latency
Typescript Handling: Creates and reads typescript files for every command
Binary Files: Several binary files (help50, http-server, make, sqlite3, valgrind) show "No diff generated" - unclear what changed
Typo: Line 116 in help50.sh has alias $name=_rhetocial (should be _rhetorical)
Overall Assessment
This is a substantial improvement that replaces a Python-based tool with a more tightly integrated shell-based solution. The new system provides real-time, context-aware help for common student errors, which should significantly improve the learning experience in CS50.****
had the AI double check everything after the built, says this is an improvement. Looking through the checks, the docker built to completion and finalized the process. I can check the tests through my codebase real quick, but everything looks good I'm building a dockerfile and testing it out remotely through a codebase to do tests in the container to double check everything. Going to take a moment to compile all the .c files as I'm building the container from scratch. |
|
@rongxin-liu Here’s a concrete way to sanity-check this branch end-to-end. Build/run:
Inside the container (in
Then try a quick matrix of the patterns the helpers are meant to catch:
For quicker spot-checks (without interactive shells), you can also pipe known error output into the helper scripts directly, e.g. If you’d like, I can also run through the full interactive matrix and paste the exact command/output transcript. |
To be squashed when merging.
This new version is implemented in Bash (instead of Python) as follows, wherein usage is inspired by
systemctl, even though it doesn't run as a daemon but, rather, per login shell. It runs locally and automatically now, without any server.help50 startsets$HELP50to$$, the PID of the shell in which the command was run and launchesscript, which logs standard I/O to/tmp/help50.$$.typescript.help50 stopsendsSIGHUPto the PID ofscript.help50 statuschecks for$HELP50, which is set only whenhelp50is started for a shell.help50 disablewrites/tmp/help50.lock.help50 enabledeletes/tmp/help50.lock.help50 is-enabledchecks for/tmp/help50.lock./etc/profile.d/help50.shis a config that that's only sourced when$HELP50is set.$PROMPT_COMMANDto_help50, which is a Bash function implemented therein that, if the most recent command exited with non-0 status (per$?), checks for/tmp/help50.$HELP50.typescript, passes it as standard input to each executable in/opt/cs50/lib/help50/(implemented in any language)._helpfulfunction that, by default, displays it in yellow to help the user._helplessinstead, which doesn't do anything incs50/clibut can be overridden incs50/codespaceto relay it to ddb50._helpedis called, which doesn't do anything incs50/clibut can be overridden incs50/codespaceto indicate to the user that help is (no longer) available./etc/profile.d/cli.shstartshelp50automatically./opt/cs50/lib/clicontains several helper functions (written in Bash) that our own wrappers andhelp50use.