Skip to content

Conversation

@cardmagic
Copy link
Owner

Summary

  • Force UTF-8 encoding in CLI executable to prevent encoding errors in C/POSIX locales
  • Bump version to 2.3.1

Problem

When running classifier in environments with non-UTF8 locales (like Homebrew's test sandbox which uses LC_ALL=C), the CLI fails with:

Error: "\xC2" on US-ASCII

Solution

Explicitly set Encoding.default_external and Encoding.default_internal to UTF-8 at CLI startup.

Test

LC_ALL=C classifier -r sms-spam-filter "test message"
# Now works instead of failing with encoding error

The CLI now explicitly sets UTF-8 encoding for external and internal
strings to prevent encoding errors when run in environments with
C/POSIX locales (e.g., Homebrew test sandboxes).

Bump version to 2.3.1
@cardmagic cardmagic force-pushed the fix-encoding-for-non-utf8-locales branch from 16149bf to 1eaca6e Compare January 1, 2026 18:29
@cardmagic cardmagic merged commit 76d3969 into master Jan 1, 2026
5 checks passed
@cardmagic cardmagic deleted the fix-encoding-for-non-utf8-locales branch January 1, 2026 18:29
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 1, 2026

Greptile Summary

Fixed encoding errors in non-UTF8 locales (like Homebrew's C/POSIX test sandbox) by forcing UTF-8 encoding in the CLI executable and bumped version to 2.3.1.

  • Added Encoding.default_external and Encoding.default_internal settings to exe/classifier
  • Bumped version from 2.3.0 to 2.3.1
  • One redundant comment should be removed per project guidelines

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Simple, focused fix that addresses a real bug in non-UTF8 locales. The encoding fix is the correct Ruby idiom for this problem, and the version bump is appropriate. Only minor style improvement suggested (removing redundant comment).
  • No files require special attention

Important Files Changed

Filename Overview
exe/classifier added UTF-8 encoding fix for non-UTF8 locales; one redundant comment
lib/classifier/version.rb version bumped to 2.3.1

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as exe/classifier
    participant Ruby as Ruby Runtime
    participant ClassifierCLI as Classifier::CLI
    
    User->>CLI: Execute classifier command
    CLI->>Ruby: Set Encoding.default_external = UTF-8
    CLI->>Ruby: Set Encoding.default_internal = UTF-8
    Note over CLI,Ruby: Forces UTF-8 encoding for C/POSIX locales
    CLI->>ClassifierCLI: require 'classifier/cli'
    CLI->>ClassifierCLI: new(ARGV).run
    ClassifierCLI-->>CLI: result hash
    CLI->>CLI: warn result[:error] unless empty?
    CLI->>CLI: puts result[:output] unless empty?
    CLI->>User: exit result[:exit_code]
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

#!/usr/bin/env ruby
# frozen_string_literal: true

# Force UTF-8 encoding for proper handling of model data and user input
Copy link
Contributor

Choose a reason for hiding this comment

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

style: comment restates what the code already shows

Suggested change
# Force UTF-8 encoding for proper handling of model data and user input

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: exe/classifier
Line: 4:4

Comment:
**style:** comment restates what the code already shows

```suggestion
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants