Skip to content

Conversation

danbev
Copy link
Member

@danbev danbev commented Jan 29, 2025

This commit updates some of JSON snippets in README.md file and
removes the json language tag from the code blocks.

The motivation for this changes is that if there is invalid json in a
code snippet these are highlighted in red which can make it somewhat
difficult to read and can be a little distracting. This can be seen here

@ngxson
Copy link
Collaborator

ngxson commented Jan 29, 2025

IMO we should not mark this as javascript because they are definitely not a valid javascript either.

On github, invalid javascript may not render as invalid syntax (highlighted in red), but will do somewhere else.

So the best way is to simply remove the json, so it will become a plain text.

This commit updates some of JSON snippets in README.md file and
removes the `json` language tag from the code blocks.

The motivation for this changes is that if there is invalid json in a
code snippet these are highlighted in red which can make it somewhat
difficult to read and can be a little distracting.
@danbev danbev force-pushed the server-readme-json-snippets branch from e227845 to fdf735e Compare January 29, 2025 19:53
@isaac-mcfadyen
Copy link
Contributor

As an alternative proposal, we could switch the language tag to jsonc and then put anything we know is invalid (as seen from that link) in comments.

Current

[
  {
    "index": 0,
    "embedding": [
      [ ... embeddings for token 0   ... ],
      [ ... embeddings for token 1   ... ],
      [ ... ]
      [ ... embeddings for token N-1 ... ],
    ]
  },
  ...
  {
    "index": P,
    "embedding": [
      [ ... embeddings for token 0   ... ],
      [ ... embeddings for token 1   ... ],
      [ ... ]
      [ ... embeddings for token N-1 ... ],
    ]
  }
]

New

[
  {
    "index": 0,
    "embedding": [
      // [ ... embeddings for token 0   ... ],
      // [ ... embeddings for token 1   ... ],
      // [ ... ]
      // [ ... embeddings for token N-1 ... ],
    ]
  },
  // ...
  {
    "index": "P",
    "embedding": [
      // [ ... embeddings for token 0   ... ],
      // [ ... embeddings for token 1   ... ],
      // [ ... ]
      // [ ... embeddings for token N-1 ... ],
    ]
  }
]

@danbev
Copy link
Member Author

danbev commented Jan 30, 2025

@isaac-mcfadyen Thanks for the suggestion about json with comments! I'll opt on the side of caution and not use a language tag to avoid any rendering issues if jsonc is not supported by a particular render. But feel free to open a PR if you feel strongly about this.

@danbev danbev merged commit e044976 into ggml-org:master Jan 30, 2025
1 check passed
@danbev danbev deleted the server-readme-json-snippets branch January 30, 2025 04:48
tinglou pushed a commit to tinglou/llama.cpp that referenced this pull request Feb 13, 2025
This commit updates some of JSON snippets in README.md file and
removes the `json` language tag from the code blocks.

The motivation for this changes is that if there is invalid json in a
code snippet these are highlighted in red which can make it somewhat
difficult to read and can be a little distracting.
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Feb 26, 2025
This commit updates some of JSON snippets in README.md file and
removes the `json` language tag from the code blocks.

The motivation for this changes is that if there is invalid json in a
code snippet these are highlighted in red which can make it somewhat
difficult to read and can be a little distracting.
mglambda pushed a commit to mglambda/llama.cpp that referenced this pull request Mar 8, 2025
This commit updates some of JSON snippets in README.md file and
removes the `json` language tag from the code blocks.

The motivation for this changes is that if there is invalid json in a
code snippet these are highlighted in red which can make it somewhat
difficult to read and can be a little distracting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants