In #2206263: drupal_is_cli() returns FALSE on PHP CLI we removed most logic of drupal_is_cli().
In #2228341: Objectify session management functions + remove session.inc SessionManager duplicated its one line logic, to keep it unit-testable.

It has no value anymore, we should just check PHP_SAPI directly.
One less drupal_* function to learn

CommentFileSizeAuthor
#4 drupal-is_cli-2294161-3.patch2.16 KBParisLiakos
#1 drupal-is_cli-2294161-1.patch2.17 KBParisLiakos
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Issue summary: View changes
Status: Active » Needs review
FileSize
drupal-is_cli-2294161-1.patch2.17 KB
ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 1: drupal-is_cli-2294161-1.patch, failed testing.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs work » Needs review
FileSize
drupal-is_cli-2294161-3.patch2.16 KB

-.-

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

Awesome.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 8074550 and pushed to 8.x. Thanks!

  • alexpott committed 8074550 on 8.x 
    Issue #2294161 by ParisLiakos: Deprecate and remove usages of...
chx’s picture

chx CreditAttribution: chx commented
Status: Fixed » Active

That was great... except now we can't implement phpdbg support in drupal_is_cli() because there's no API function to fix.

chx’s picture

chx CreditAttribution: chx commented

alexpott asked for clarification. phpdbg is a php debugger that has been included in PHP 5.6. https://wiki.php.net/rfc/phpdbg

Like CLI, and gdb, phpdbg is an executable intended to be executed at the terminal in an interactive way.

the way to support it is PHP_SAPI === 'cli' || PHP_SAPI === 'phpdbg' but now there's no place to put this.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented

i do not see why we should specialcase for phpdbg.
Did you try something on it and it didnt work?
If there is an issue with phpdbg support, and we need to add checks like that, then an issue should be opened, which adds a method to the Enviroment component.
Keeping drupal_is_cli() around just in case we might need it, is something i disagree with

chx’s picture

chx CreditAttribution: chx commented
Status: Active » Fixed

w/e

jbrown’s picture

jbrown CreditAttribution: jbrown commented

The change record is not correct - drupal_is_cli() hasn't been deprecated, it has been removed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

fubhy’s picture

fubhy CreditAttribution: fubhy commented

Sorry, but why exactly was this issue closed? It's impossible currently to run the Drupal testsuite with phpdbg. Checking just for 'cli' is simply wrong if we want to support phpdbg and I am pretty sure we want that.

cilefen’s picture

cilefen CreditAttribution: cilefen as a volunteer commented
Related issues: +#2904700: Do not (only) use PHP_SAPI to determine CLI mode

It was closed for having met its goals at the time.

cilefen’s picture

cilefen CreditAttribution: cilefen as a volunteer commented

Plus it's been closed so long, your follow up issue is fine.