Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give TERM=dumb Higher Priority #88

Closed
plroebuck opened this issue Dec 1, 2018 · 7 comments
Closed

Give TERM=dumb Higher Priority #88

plroebuck opened this issue Dec 1, 2018 · 7 comments

Comments

@plroebuck
Copy link
Contributor

Emacs M-x shell runs the shell in an environment where the TERM variable is set to dumb, which informs the shell and applications that the terminal does not support any terminal control escape sequences.

While you have code to handle the dumb setting, both the Windows platform processing and the TERM_PROGRAM are processed first.

For instance on macOS,

$ cd /path/to/mocha/node_modules
$ TERM=dumb node -e "var tty = require('./supports-color'); console.log(tty.stdout);"
{ level: 2, hasBasic: true, has256: true, has16m: false }
$ TERM_PROGRAM= TERM=dumb node -e "var tty = require('./supports-color'); console.log(tty.stdout);"
false

Mocha is getting publicly blamed for this. Suggestions?

@Qix-
Copy link
Member

Qix- commented Dec 1, 2018

I agree. Using TERM=dumb should absolutely be of higher priority than the terminal program used as it shows user intent.

@Alhadis
Copy link
Contributor

Alhadis commented Dec 1, 2018

There's another (more reliable) variable we can check too; see my reply at mochajs/mocha#1304 (comment)

Happy to submit a PR for this, as it'll address the issue at SO as well. But I agree TERM=dumb needs to have higher importance; if a user's requested that no terminal codes be emitted, they damn mean it.

@Qix-
Copy link
Member

Qix- commented Dec 2, 2018

PR is welcome :)

@plroebuck
Copy link
Contributor Author

I concur with PR change from @Alhadis.
Mocha will be doing a major release RSN; it'd be nice to get this in (via package dependency update).

@Alhadis
Copy link
Contributor

Alhadis commented Dec 2, 2018

A breaking-change release? 😀

Could we shove a new describe() function for BDD style testing that's named when()....?

@Alhadis
Copy link
Contributor

Alhadis commented Dec 2, 2018

λ AdobeFontMetrics.js (master): t
npx mocha test/*-spec.js

  When parsing global font information,
    ✓ It stores format and version headers
    ✓ It stores `MetricsSets`
    ✓ It stores `FontName`
    ✓ It stores `FullName`
    ✓ It stores `FamilyName`
    ✓ It stores `Weight`
    ✓ It stores `FontBBox`
    ✓ It stores `Version`
    ✓ It stores `Notice`
    ✓ It stores `EncodingScheme`
    ✓ It stores `MappingScheme`
    ✓ It stores `EscChar`
    ✓ It stores `CharacterSet`
    ✓ It stores `Characters`
    ✓ It stores `IsBaseFont`
    ✓ It stores `VVector`
    ✓ It stores `IsFixedV`
    ✓ It stores `CapHeight`
    ✓ It stores `XHeight`
    ✓ It stores `Ascender`
    ✓ It stores `Descender`
    ✓ It stores user-defined fields
    When parsing ACFM data,
      ✓ It stores `Descendents`
    When parsing AMFM data,
      ✓ It stores `Masters`
      ✓ It stores `Axes`

  When parsing direction-specific properties,
    When direction 0 is specified,
      ✓ stores `UnderlinePosition`
      ✓ stores `UnderlineThickness`
      ✓ stores `ItalicAngle`
      ✓ stores `CharWidth`
      ✓ stores `IsFixedPitch`
    When direction 1 is specified,
      ✓ stores `UnderlinePosition`
      ✓ stores `UnderlineThickness`
      ✓ stores `ItalicAngle`
      ✓ stores `CharWidth`
      ✓ stores `IsFixedPitch`
    When direction 2 is specified,
      ✓ stores `UnderlinePosition`
      ✓ stores `UnderlineThickness`
      ✓ stores `ItalicAngle`
      ✓ stores `CharWidth`
      ✓ stores `IsFixedPitch`
    When no direction is specified,
      ✓ uses an implied direction of 0
    When multiple directions are specified,
      ✓ assigns each their respective fields
      ✓ merges duplicate declarations
      ✓ ignores empty blocks

  When parsing character metrics,
    ✓ initialises fields with empty or zeroed values
    ✓ stores character codes in decimal
    ✓ stores character codes in hexadecimal
    ✓ stores character names
    ✓ stores horizontal character widths
    ✓ stores horizontal character heights
    ✓ stores vertical character widths
    ✓ stores vertical character heights
    ✓ can simultaneously set horizontal metrics
    ✓ can simultaneously set vertical metrics
    ✓ can simultaneously set homogenous metrics
    ✓ stores character bounding boxes
    ✓ stores ligature sequences

  When parsing track kerning data,
    ✓ stores it as a flat array
    ✓ initialises each field with zero

  When parsing pair-wise kerning data,
    ✓ stores kerning pairs for horizontal writing
    ✓ stores kerning pairs for vertical writing
    ✓ stores kerning pairs for both writing directions
    ✓ can specify characters by codepoint instead of name
    ✓ can specify adjustments along the X axis only
    ✓ can specify adjustments along the Y axis only

  When parsing composite characters,
    ✓ stores them by name
    ✓ composes them from named characters

  When parsing PostScript literals,
    When parsing strings,
      ✓ parses simple values
      ✓ parses empty values
      ✓ parses multiple values
      ✓ parses values containing newlines
      ✓ parses values containing special characters
      ✓ parses values containing parentheses
      ✓ parses values containing nested parentheses
      ✓ parses unterminated values
    When parsing arrays,
      ✓ parses empty arrays
      ✓ parses multiple arrays
      ✓ parses arrays of numbers
      ✓ parses arrays of strings
      ✓ parses arrays of booleans
      ✓ parses arrays of names
      ✓ parses nested arrays
      ✓ parses unterminated arrays

  83 passing (72ms)

@Alhadis
Copy link
Contributor

Alhadis commented Dec 2, 2018

Have a look for yourself: https://github.com/Alhadis/AdobeFontMetrics.js/blob/master/test/1.1-global-spec.js

I actually edited the the it handler to literally include "It" before the test title in the output,because I was getting sick of my beautifully-writen tests not sounding as eloquent as I am.

I'm tired as shit right now, just letting everybbody be aware of that. :D

plroebuck added a commit to plroebuck/supports-color that referenced this issue Dec 3, 2018
Rearranges evaluation order so check for dumb terminal precedes other terminal and platform-specific
checks. Will allow Emacs shell to function properly.

Fixes chalk#88
sindresorhus pushed a commit that referenced this issue Dec 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants