ethan-ou / vscode-gift

Moodle's GIFT format extension for VSCode, including syntax highlighting, error checking and snippets.
https://ethan-ou.github.io/vscode-gift-docs/
MIT License
19 stars 2 forks source link

Problem with \n not correctly handled in preview #7

Open didoudiaz opened 3 years ago

didoudiaz commented 3 years ago

The escape character \n should be reflected as a newline in the Preview window.

Here is an example (type this GIFT question and look at the preview) :

Is the newline escape character\nworking ? {F}

NB: I have modified a bit the question (no newline after the \n).

ethan-ou commented 3 years ago

Ah it really isn't working! Could you give me a screenshot of what the question looks like when it's imported into Moodle?

didoudiaz commented 3 years ago

Here is the screenshot inside VScode:

image

And here is the screenshot of the question inside moodle (\n is treated as <br>):

Capture d’écran 2020-10-30 à 11 42 36
fuhrmanator commented 3 years ago

@ethan-ou If this turns out to be a problem with the parser, I can try to fix it. I'm thinking a new test needs to include that input to make sure it's parsed correctly.

ethan-ou commented 3 years ago

@fuhrmanator I believe it might be because the preview just takes the raw output from the parser. I'm not 100% sure how new lines would work when converted to HTML but if not, I can write in some parsing logic to add new lines in the preview plugin.

Thanks for picking this up by the way! I was going to add an issue on your project when I got around to it.

fuhrmanator commented 3 years ago

@didoudiaz I think I fixed it in the previewer at https://fuhrmanator.github.io/GIFT-grammar-PEG.js/editor/editor.html

@ethan-ou perhaps you can use that logic to change your extension's preview?

ethan-ou commented 3 years ago

@fuhrmanator had a read of your code and found an interesting edge case. @didoudiaz used a single backslash \n whereas you were using a double backslash \\n. The thing is, they both translate to \n when entered into GIFT.

GIFT newline

In order to get \\n after parsing, you'd need to type in \\\n or \\\\n in the GIFT document.

I'm thinking the regex should probably be:

switch (giftText.format) {
        case "html":
        case "moodle":
            // convert Moodle's embedded line feeds (GIFT) to HTML
            unescapedString = giftText.text.replace(/\n/g, '<br\>');
            html = unescapedString;
            break;
        case "plain":
            html = giftText.text;
            break;

        case "markdown":
            html = converter.makeHtml(giftText.text, { breaks: true });
            break;

        default:
            break;
    }

Also, this fix could be extended to carriage line returns \r\n pretty easily, so we'd end up with:

switch (giftText.format) {
        case "html":
        case "moodle":
            // convert Moodle's embedded line feeds (GIFT) to HTML
            unescapedString = giftText.text.replace(/(?:\r\n|\r|\n)/g, '<br>');
            html = unescapedString;
            break;
        case "plain":
            html = giftText.text;
            break;

        case "markdown":
            html = converter.makeHtml(giftText.text, { breaks: true });
            break;

        default:
            break;
    }
ethan-ou commented 3 years ago

@didoudiaz the preview plugin should be updated. Give it a try and see whether everything works on your end.

fuhrmanator commented 3 years ago

I didn't try your change yet, but wanted to clarify that the \\n in a regex is because the first \ escapes the second \ so that it actually matches a \n (two characters) as opposed to a carriage return:

image

fuhrmanator commented 3 years ago

I updated the preview extension (reload) and it says 0.0.9. I don't think it's working on my VSCode: image

ethan-ou commented 3 years ago

@fuhrmanator my apologies you were right about escaping the backslashes, and it looks like enabling line breaks in markedjs does something different to I imagined. v0.0.10 is up now if you want to try again.

image

fuhrmanator commented 3 years ago

Sorry for the noise. Looking again at the definition for GIFT, I believe this could be a parser issue:

\n Places a newline in question text -- blank lines delimit questions

I totally ignored this case in the parser (it actually filters out whitespace, see here), but I'm wondering if that's the right approach. That is, should the parser convert the \n (two character token) into a real newline (one character)?

It's easy enough to do (I just added the rule in the grammar, but I will have to clean up that white space logic). A lot of tests will break as written, but that's normal.

If you agree, @ethan-ou, let's open an issue there - I think it changes a lot (especially previews), so I would do a branch.

ethan-ou commented 3 years ago

Sounds good. I opened an issue on your repo so we'll take it from there.

didoudiaz commented 3 years ago

Not yet sure if it is a parser-issue (or a client application issue, like the VSCode plugin). See my comment in the newly created parser issue.

didoudiaz commented 3 years ago

I "re-import" your remark in this thread to separate things.

I'm adding a quick action to the extension that will help you escape things much faster. Just select the block of code you'd like to escape and the extension will handle it for you. You'll be able to access it through the lightbulb icon (quick actions) or if you select the code, right click > > refactor > escape special characters. I'd upload a preview but my internet's going nuts today.

Wonderful ! Thanks How can I update the plugin inside VSCode to test this beta functionnality ?

BTW, do you plan to add other functionalities ? Here are some ideas:

  1. Replace all real newlines by \n. The result is a single (possibly long) line.
  2. Replace all \nby real newlines (the reverse action)
  3. In a selected block, replace spaces by &nbsp;
  4. In a selected block, replace each &nbsp; by a space (reverse action)
  5. In a selected block, replace a leading space by &nbsp; (only if the line starts with a space)
  6. indent/desindent a block (move 4 spaces forwards/backwards the block) I don't know if this should be in your plugin or provided by VSCode.

Point 5 is because moodle seems to ignore spaces at the begining of a line when importing a GIFT file (at least on our moodle)... replacing the fisrt space by &nbsp; works. e.g.:

[html]What is returned by mystery(2) ?
<span class\="" style\="font-family\: monospace;color\: blue;white-space\: pre-wrap;">
int mystery(int x) \{
&nbsp;   switch (x) \{
&nbsp;      case 1\: x \= 4;
&nbsp;      case 2\: x \= 5;
&nbsp;      case 3\: x \= 6;
&nbsp;   \}
&nbsp;   return x;
\}
</span>

What do you think ?

ethan-ou commented 3 years ago

@didoudiaz if you restart your Vscode it should be working by now. By the time I'm writing this you might already see it.

The actions I'm most inclined to implement right now is the ability to switch question types (e.g. multiple choice to short answer or matching) and generated answer weights for multiple choice multiple answer questions. I want to make sure the most general use cases are handled, especially for people who aren't as technically savvy.

I also don't want too many actions so that the context menus stay relatively small. Wouldn't want to go through a long menu just to find the action you're looking for.

Otherwise, do you think the actions you're suggesting would be useful for other users too?

didoudiaz commented 3 years ago

I also don't want too many actions so that the context menus stay relatively small. Wouldn't want to go through a long menu just to find the action you're looking for.

This makes sense.

Otherwise, do you think the actions you're suggesting would be useful for other users too?

My suggestions are really useful for people wanting to insert "verbatim text" ("preformated" in the html jargon) in their questions. Such a text should be reflected as is once imported in moodle. It is the case of source code (questions about programming) but also of examples. In that case, the user will surely use [html] and surround the text with html blocks like <pre>...</pre> (depending on what their plateform recognizes, maybe <code>...</code>or <span>...</span>). It is thus much easier to format the text inside the editor and have moodle reflect it exactly. However, as I said you have to be aware about:

A help for these tasks is a really improvement.

fuhrmanator commented 3 years ago

VSCode snippets might be a good compromise? https://code.visualstudio.com/docs/editor/userdefinedsnippets Otherwise, some regex replacement pattern?

ethan-ou commented 3 years ago

@fuhrmanator @didoudiaz I think I found a good solution using commands. To access them, you can use CTRL + SHIFT + P and get them from the dialog menus. There's more friction to the process as a user but it means we can implement many specific formatting actions without overwhelming other users.

When I get to it, I'll try the real new line action first. I think it'll be good to discuss the HTML ones in greater detail.

didoudiaz commented 3 years ago

Both snippets and commands seem promising. I'm a new VSCode user, so I don't yet master it and know all its possibility. I'm a bit overwhelmed with remote-teaching duties at my University but I'm very interested by your ideas.

fuhrmanator commented 3 years ago

I followed https://code.visualstudio.com/docs/editor/userdefinedsnippets#_create-your-own-snippets to make the following snippet in a gift.json file (I selected gift format in the option to make my own snippet):

{
    // Place your snippets for gift here. Each snippet is defined under a snippet name and has a prefix, body and 
    // description. The prefix is what is used to trigger the snippet and the body will be expanded and inserted. Possible variables are:
    // $1, $2 for tab stops, $0 for the final cursor position, and ${1:label}, ${2:another} for placeholders. Placeholders with the 
    // same ids are connected.
    // Example:
    "Preformatted Code in HTML formatted question": {
        "prefix": "preformatted code in HTML formatted question",
        "body": [
            "<span class\\=\"\" style\\=\"font-family\\: monospace;color\\: blue;white-space\\: pre-wrap;\">",
            "int mystery(int x) \\{",
            "&nbsp;   return x;",
            "\\\\}",
            "</span>"
        ],
        "description": "Log output to console"
    }
}

Then, as I type a question, like:

[html]Here's a sample of code:
pre

as the pre is typed, VSCode recognizes the snippet and I can select it to complete. This example isn't using much power (no arguments like $1, $2) but gets the <span> tag and at least one line that is indented, with some escaping of special characters. Note, you have to DOUBLE escape since in JSON the \ is an escape character.

To be honest, I don't understand why the next-to-last line with "\\}" requires 4 backslashes! I tried it with only two, but it didn't work when the snippet gets inserted.

didoudiaz commented 3 years ago

@fuhrmanator Thank you, seems very promising ! I could also define a snippet form my examples following the same line.

didoudiaz commented 3 years ago

Hi, I have discovered a new problem with backslash (maybe it would be better in another issue, I don't know). Using \{ \} in the answers causes a problem to the parser. Try with:

b+ is equivalent to
{
~b\{2,\}
=bb*
}

I obtain an error Expected } but end of input found. gift[4,5] The on-line GIFT validator accepts it.

ethan-ou commented 3 years ago

I made a fix to patch it. It should be live now.

Keep coming if you have any more issues!