Fix #124 according to the snippet by @mrvdb #128

qwazix · 2019-06-25T18:56:07Z

I changed the sh alias to shell instead of bash.

The additions to the highlight(nodes) function look redundant.
It works for me without them but maybe they cover an edge case I
cannot think about?

I changed the sh alias to shell instead of bash. The additions to the `highlight(nodes)` function look redundant. It works for me without them but maybe they cover an edge case I cannot think about?

mrvdb · 2019-06-26T08:02:47Z

Notes for consideration:

sh is an alias of bash in highlightjs itself, perhaps not deviate from their maps (see : https://github.com/highlightjs/highlight.js/blob/master/src/languages/bash.js#L40 ) I think they interpret shell as a console session which has a different begin pattern than sh, but otherwise just uses bash highlighting.
the registerlanguage parts are to make sure the used language is registered properly (see highlightjs documentation on 'usage') as we load 'on demand' What it does internally, I'm not sure.

qwazix · 2019-06-26T18:32:43Z

Ok I will revert sh to bash
I assume it loads it in the list of supported languages for auto-detection. Since we don't seem to do auto-detection (correct me if I'm wrong) it is probably redundant. I will try to see what it does but highlighting specific code blocks works without it. (There was no registerLanguage call before anyway).

qwazix · 2019-06-26T18:49:07Z

Regarding registerLanguage it doesn't have anything to do with auto-detection. In the documentation it is referenced only under CommonJS and not for inclusion in the browser. In the following snippet both auto-detection and specific block highlighting works so I'm inclined to keep the registerLanguage part out unless there's a demonstrable lack of functionality.

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
    <link rel="stylesheet" href="//cdnjs.cloudflare.com/ajax/libs/highlight.js/9.15.8/styles/default.min.css">
    <script src="highlight/highlight.pack.js" ></script>
    <script src="highlight/javascript.min.js" ></script>
</head>
<body>
    <pre><code>
        document.querySelectorAll("body.dark");
    </code></pre>
    <!-- <script>hljs.initHighlightingOnLoad();</script> -->
    <script>hljs.highlightBlock(document.querySelector("code"));</script>
</body>
</html>

because this is the alias in highlight itself. (see #128 (comment))

mrvdb · 2019-06-27T06:14:50Z

I'm inclined to keep the registerLanguage part out

Agreed. I can't see any useful effect either, I assumed it at least kept the list of languages, but it does not. While we're cleaning, the langs variable and its usage seems redundant too.

qwazix · 2019-06-27T15:15:12Z

Nice catch. Removed it.

This solves the error 500 on the /api/me endpoint. Replace token search query `=` with `LIKE` to fix sqlite complaining about no valid tokens. Also checked with MySQL and it still works after the change.

Previously, this would only run when configuring an instance for single-user usage. Now it'll also run when configuring for multi-user usage. It also adds a log when the database has already been initialized.

do anything useful (see #128 (comment))

thebaer · 2019-06-27T19:05:53Z

Thanks, @qwazix and @mrvdb. This looks great! Merging now.

Fix #124 according to the snippet by @mrvdb

c2d7c2c

I changed the sh alias to shell instead of bash. The additions to the `highlight(nodes)` function look redundant. It works for me without them but maybe they cover an edge case I cannot think about?

Change sh alias back to bash

0842119

because this is the alias in highlight itself. (see #128 (comment))

Michael Demetriou and others added 3 commits June 27, 2019 18:15

Fix #96

a102f97

This solves the error 500 on the /api/me endpoint. Replace token search query `=` with `LIKE` to fix sqlite complaining about no valid tokens. Also checked with MySQL and it still works after the change.

Always initialize database after --config

8d9f60a

Previously, this would only run when configuring an instance for single-user usage. Now it'll also run when configuring for multi-user usage. It also adds a log when the database has already been initialized.

Remove langs list from post-render as it does not actually

4feac6d

do anything useful (see #128 (comment))

thebaer added this to the 0.10 milestone Jun 27, 2019

thebaer approved these changes Jun 27, 2019

View reviewed changes

thebaer merged commit f26e0ca into develop Jun 27, 2019

thebaer deleted the fix-c-syntax-highlighting branch June 27, 2019 19:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #124 according to the snippet by @mrvdb #128

Fix #124 according to the snippet by @mrvdb #128

qwazix commented Jun 25, 2019

mrvdb commented Jun 26, 2019

qwazix commented Jun 26, 2019 •

edited

qwazix commented Jun 26, 2019

mrvdb commented Jun 27, 2019 •

edited

qwazix commented Jun 27, 2019

thebaer commented Jun 27, 2019

Fix #124 according to the snippet by @mrvdb #128

Fix #124 according to the snippet by @mrvdb #128

Conversation

qwazix commented Jun 25, 2019

mrvdb commented Jun 26, 2019

qwazix commented Jun 26, 2019 • edited

qwazix commented Jun 26, 2019

mrvdb commented Jun 27, 2019 • edited

qwazix commented Jun 27, 2019

thebaer commented Jun 27, 2019

qwazix commented Jun 26, 2019 •

edited

mrvdb commented Jun 27, 2019 •

edited