Skip to content

Conversation

@russell
Copy link
Contributor

@russell russell commented Jan 25, 2021

They are very similar gems, but the former isn't supported on newer
versions of OSX. Basically you'll see an error like.

gh-markdown.c:56:29: error: implicitly declaring library function 'isspace' with type 'int (int)' [-Werror,-Wimplicit-function-declaration]
                while (i < lang->size && !isspace(lang->data[i]))
                                          ^
gh-markdown.c:56:29: note: include the header <ctype.h> or explicitly provide a declaration for 'isspace'

Since the github-markdown gem is deprecated, we need to change gems.

Risks

  • Low: anything that could go wrong will be covered in tests

@russell russell requested a review from grosser as a code owner January 25, 2021 15:46
@russell russell requested a review from a team January 25, 2021 15:46
They are very similar gems, but the former isn't supported on newer
versions of OSX.  Basically you'll see an error like.

```
gh-markdown.c:56:29: error: implicitly declaring library function 'isspace' with type 'int (int)' [-Werror,-Wimplicit-function-declaration]
                while (i < lang->size && !isspace(lang->data[i]))
                                          ^
gh-markdown.c:56:29: note: include the header <ctype.h> or explicitly provide a declaration for 'isspace'
```

Since the github-markdown gem is deprecated, we need to change gems.
@russell
Copy link
Contributor Author

russell commented Jan 26, 2021

@grosser i think we could actually trim this down a bit and only use the https://github.com/gjtorikian/commonmarker instead of the github wrapper, they turn on some options but we could do the same instead of using the gem?

https://github.com/github/markup/blob/cd01f9ec87c86ce5a7c70188a74ef40fc4669c5b/lib/github/markup/markdown.rb#L7-L10

Let me know what you'd prefer?

@grosser
Copy link
Contributor

grosser commented Jan 26, 2021

it's only used to display the risks, so if it renders a bit wrong that should not be an issue, just make sure it cannot be used for xss (like <script> being escaped etc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants