Skip to content

Conversation

@dmalan
Copy link
Member

@dmalan dmalan commented May 9, 2024

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 start sets $HELP50 to $$, the PID of the shell in which the command was run and launches script, which logs standard I/O to /tmp/help50.$$.typescript.
  • help50 stop sends SIGHUP to the PID of script.
  • help50 status checks for $HELP50, which is set only when help50 is started for a shell.
  • help50 disable writes /tmp/help50.lock.
  • help50 enable deletes /tmp/help50.lock.
  • help50 is-enabled checks for /tmp/help50.lock.

  • /etc/profile.d/help50.sh is a config that that's only sourced when $HELP50 is set.
    • It sets $PROMPT_COMMAND to _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).
    • If any of those echo output, it passes it to a _helpful function that, by default, displays it in yellow to help the user.
    • It none of those echo output, it passes the failed command's typescript to _helpless instead, which doesn't do anything in cs50/cli but can be overridden in cs50/codespace to relay it to ddb50.
    • Else if the most recent command exited successfully, _helped is called, which doesn't do anything in cs50/cli but can be overridden in cs50/codespace to indicate to the user that help is (no longer) available.
  • /etc/profile.d/cli.sh starts help50 automatically.
  • /opt/cs50/lib/cli contains several helper functions (written in Bash) that our own wrappers and help50 use.

$ git fetch
$ git checkout help50
$ make build
$ make run

/mnt/ $ help50 start

/mnt/ $ ps f
  PID TTY      STAT   TIME COMMAND
    1 pts/0    Ss     0:00 bash --login
   24 pts/0    S      0:00 /bin/bash /opt/cs50/bin/help50 start
   26 pts/0    S+     0:00  \_ script --append --command bash --login ; exit 1 --flush --quiet --return /tmp/help50.1.typescript
   27 pts/1    Ss     0:00      \_ sh -c bash --login ; exit 1
   28 pts/1    S      0:00          \_ bash --login
  143 pts/1    R+     0:00              \_ ps f

/mnt/ $ help50 stop

Session terminated, killing shell... ...killed.

/mnt/ $ ps f
  PID TTY      STAT   TIME COMMAND
    1 pts/0    Ss     0:00 bash --login
  178 pts/0    R+     0:00 ps f

@dmalan dmalan marked this pull request as ready for review October 5, 2024 23:51
@phuyalgaurav

This comment was marked as off-topic.

Copy link

@drqsatoshi drqsatoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR #210 is still waiting for approval by an approver who has write access, btw.

@drqsatoshi
Copy link

All checks passed, still need 1 approving review by you @rongxin-liu . Looks like all checks passed.

@rongxin-liu
Copy link
Contributor

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?

Copy link

@drqsatoshi drqsatoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major Changes

  1. Help50 Installation Method

    Removed: Python-based help50 package (line 262 in Dockerfile)
    Added: Shell-based implementation with new dependencies:
    colorized-logs
    expect
    file
    fzf

  2. 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
  1. 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

  2. Improved User Check

    Changed from whoami to id -u for checking if user is root (more reliable)

  3. 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.****

@drqsatoshi
Copy link

drqsatoshi commented Jan 5, 2026

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?

.github/workflows/main.yml

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.

Copy link

@rongxin-liu Here’s a concrete way to sanity-check this branch end-to-end.

Build/run:

  • make build
  • make run

Inside the container (in /mnt):

  • Confirm help50 is enabled/starts: help50 is-enabled and help50 status
  • If needed: help50 start (and later help50 stop)

Then try a quick matrix of the patterns the helpers are meant to catch:

  • bash: run a typo like 1s (suggest ls), try ./foo.c after creating foo.c (suggest make foo then ./foo)
  • cd: create nested dirs (mkdir -p foo/bar) then cd bar from the wrong place (should suggest the right parent)
  • make: create foo/bar/baz.c then from /mnt run make baz (should suggest cd foo/bar first)
  • clang: compile a helper C file without main to trigger “undefined reference to main
  • python: create cs50.py then run python -c "from cs50 import get_int" (should warn about shadowing)

For quicker spot-checks (without interactive shells), you can also pipe known error output into the helper scripts directly, e.g. printf '%s\n' "cd: bar: No such file or directory" | /opt/cs50/lib/help50/cd.

If you’d like, I can also run through the full interactive matrix and paste the exact command/output transcript.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants