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

Treat JSON with comments as a separate language #4171

Merged
merged 8 commits into from
Jul 6, 2018
Merged

Treat JSON with comments as a separate language #4171

merged 8 commits into from
Jul 6, 2018

Conversation

Alhadis
Copy link
Collaborator

@Alhadis Alhadis commented Jun 22, 2018

This PR adds first-class support for JSON files which permit the use of inline comments (e.g., .babelrc.json, Sublime Text configs, etc). Fixes #4153.

(The .eslintrc.json sample I added is my own, in case its origin is of interest).

/cc @lildude, @Haroenv

Checklist:

  • I am associating a language with a new file extension.

  • I am adding a new language.

  • I am fixing a misclassified language

    • I have included a new sample for the misclassified language:
      • Sample source(s):
        • [URL to each sample source, if applicable]
      • Sample license(s):
    • I have included a change to the heuristics to distinguish my language from others using the same extension.
  • I am changing the source of a syntax highlighting grammar

  • I am adding new or changing current functionality

    • I have added or updated the tests for the new or changed functionality.

@Alhadis
Copy link
Collaborator Author

Alhadis commented Jun 22, 2018

https://travis-ci.org/github/linguist/jobs/395399480#L1528-L1530

   1) Failure:
 TestPedantic#test_language_names_are_sorted [/home/travis/build/github/linguist/test/test_pedantic.rb:9]:
 JSON5 should come after JSON with comments

... how on Earth does that work? 😂

Force-pushing amended commit, round 2.

filenames:
- ".babelrc"
- ".eslintrc.json"

Choose a reason for hiding this comment

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

There's also tsconfig.json used by TypeScript.

The default tsconfig.json generated by TypeScript (tsc --init)
{
  "compilerOptions": {
    /* Basic Options */
    "target": "es5",                          /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017','ES2018' or 'ESNEXT'. */
    "module": "commonjs",                     /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */
    // "lib": [],                             /* Specify library files to be included in the compilation. */
    // "allowJs": true,                       /* Allow javascript files to be compiled. */
    // "checkJs": true,                       /* Report errors in .js files. */
    // "jsx": "preserve",                     /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */
    // "declaration": true,                   /* Generates corresponding '.d.ts' file. */
    // "declarationMap": true,                /* Generates a sourcemap for each corresponding '.d.ts' file. */
    // "sourceMap": true,                     /* Generates corresponding '.map' file. */
    // "outFile": "./",                       /* Concatenate and emit output to single file. */
    // "outDir": "./",                        /* Redirect output structure to the directory. */
    // "rootDir": "./",                       /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
    // "composite": true,                     /* Enable project compilation */
    // "removeComments": true,                /* Do not emit comments to output. */
    // "noEmit": true,                        /* Do not emit outputs. */
    // "importHelpers": true,                 /* Import emit helpers from 'tslib'. */
    // "downlevelIteration": true,            /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */
    // "isolatedModules": true,               /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */

    /* Strict Type-Checking Options */
    "strict": true,                           /* Enable all strict type-checking options. */
    // "noImplicitAny": true,                 /* Raise error on expressions and declarations with an implied 'any' type. */
    // "strictNullChecks": true,              /* Enable strict null checks. */
    // "strictFunctionTypes": true,           /* Enable strict checking of function types. */
    // "strictPropertyInitialization": true,  /* Enable strict checking of property initialization in classes. */
    // "noImplicitThis": true,                /* Raise error on 'this' expressions with an implied 'any' type. */
    // "alwaysStrict": true,                  /* Parse in strict mode and emit "use strict" for each source file. */

    /* Additional Checks */
    // "noUnusedLocals": true,                /* Report errors on unused locals. */
    // "noUnusedParameters": true,            /* Report errors on unused parameters. */
    // "noImplicitReturns": true,             /* Report error when not all code paths in function return a value. */
    // "noFallthroughCasesInSwitch": true,    /* Report errors for fallthrough cases in switch statement. */

    /* Module Resolution Options */
    // "moduleResolution": "node",            /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */
    // "baseUrl": "./",                       /* Base directory to resolve non-absolute module names. */
    // "paths": {},                           /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */
    // "rootDirs": [],                        /* List of root folders whose combined content represents the structure of the project at runtime. */
    // "typeRoots": [],                       /* List of folders to include type definitions from. */
    // "types": [],                           /* Type declaration files to be included in compilation. */
    // "allowSyntheticDefaultImports": true,  /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */
    "esModuleInterop": true                   /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */
    // "preserveSymlinks": true,              /* Do not resolve the real path of symlinks. */

    /* Source Map Options */
    // "sourceRoot": "./",                    /* Specify the location where debugger should locate TypeScript files instead of source locations. */
    // "mapRoot": "./",                       /* Specify the location where debugger should locate map files instead of generated locations. */
    // "inlineSourceMap": true,               /* Emit a single file with source maps instead of having a separate file. */
    // "inlineSources": true,                 /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */

    /* Experimental Options */
    // "experimentalDecorators": true,        /* Enables experimental support for ES7 decorators. */
    // "emitDecoratorMetadata": true,         /* Enables experimental support for emitting type metadata for decorators. */
  }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, thanks! :D I've added a shorter copy, taken from yasoonOfficial/botbuilder-stride-bot (MIT licensed).

Copy link
Contributor

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

I've left two comments below, but LGTM otherwise!

Thanks @Alhadis!

extensions:
- ".json5"
- ".jsonc"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, you're right. 😅 Forgot to check the usage of jsonc.

Fixed!

- ".jslintrc"
- "tsconfig.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

Double quotes are not necessary here since the filename doesn't start with a dot.

@Alhadis
Copy link
Collaborator Author

Alhadis commented Jul 1, 2018

Two more filenames added. Both samples taken from the links below:

(That's not a typo of "JS Lint", BTW)

@Alhadis
Copy link
Collaborator Author

Alhadis commented Jul 2, 2018

@lildude Any idea what I'm doing wrong? 😞 Either I'm stupid and/or I'm missing something obvious here...

- ".tern-config"
- ".tern-project"
- composer.lock
- mcmod.info
language_id: 174
JSON5:
JSON with comments:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we consider this a title and use title case (so, JSON with Comments)? It feels weird every time I open this pull request...

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @pchaigno: this needs to be title case.

@pchaigno
Copy link
Contributor

pchaigno commented Jul 2, 2018

Any idea what I'm doing wrong? 😞 Either I'm stupid and/or I'm missing something obvious here...

There's still a .jscsrc file in the JSON directory; could that be it?

@Alhadis
Copy link
Collaborator Author

Alhadis commented Jul 2, 2018

I swear I already nuked that...

@Alhadis
Copy link
Collaborator Author

Alhadis commented Jul 5, 2018

Caps fixed, folks! 😁

@nathanshelly nathanshelly mentioned this pull request Feb 10, 2020
3 tasks
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add jsonc (json with comments)
4 participants