Use stream_isatty for CLI stdin detection#240
Merged
Conversation
bin/djot called posix_isatty(STDIN) for stdin/tty detection, but ext-posix is not a hard dependency of the package. On PHP builds without it (Windows, minimal installs) the CLI fatals with 'undefined function posix_isatty()' instead of converting. Replace both calls with the built-in stream_isatty(), available in core since PHP 7.2 and requiring no extension. Add CLI tests that run with posix_isatty disabled to cover the convert subcommand and legacy stdin.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #240 +/- ##
=========================================
Coverage 91.89% 91.89%
Complexity 3527 3527
=========================================
Files 106 106
Lines 9970 9970
=========================================
Hits 9162 9162
Misses 808 808 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
bin/djotusesposix_isatty(STDIN)to detect whether stdin is a terminal (for the interactive hint and the legacy bare-stdin convert path).ext-posixis not a hard dependency of the package, so on PHP builds without it - Windows, or minimal/container PHP installs - the CLI fatals:instead of converting the input.
Fix
Replace both
posix_isatty(STDIN)calls with the built-instream_isatty(STDIN), which is part of PHP core since 7.2 and requires no extension. Behavior is identical where posix was available, and now works everywhere.Added two CLI tests that run
bin/djotwithposix_isattydisabled (-d disable_functions=posix_isatty), covering both theconvert -subcommand and the legacy bare-stdin form.Context
Surfaced while updating the djot-intellij plugin to call this CLI: the plugin needs a fallback specifically because of this crash. With this fix the CLI is safe to invoke on any supported PHP build.
Note
phpstanreports one pre-existing error inParser/BlockParser.php:589(offsetAccess.invalidOffset) on master, unrelated to this change - left untouched.