add Paste code to submit feature #2720

as6325400 · 2024-09-23T09:03:41Z

Modify PR merge to main

vmcj · 2024-09-25T06:20:11Z

Hi,

I think the rebasing still didn't work correctly, for example 92e3ad7 should stay specific for the 8.3 branch.

Can you run:

git remote add upstream https://github.com/DOMjudge/domjudge
git fetch upstream
git rebase -i upstream/main

And only use your own commits there? Alternatively you enabled the option that we can do this, if you want I can try it later this week if you're fine with it.

as6325400 · 2024-09-25T08:00:36Z

Hi,

I think the rebasing still didn't work correctly, for example 92e3ad7 should stay specific for the 8.3 branch.

Can you run:
git remote add upstream https://github.com/DOMjudge/domjudge
git fetch upstream
git rebase -i upstream/main
And only use your own commits there? Alternatively you enabled the option that we can do this, if you want I can try it later this week if you're fine with it.

I think I've got it right now. Could you please take a look?

Thanks!

vmcj · 2024-09-27T06:39:33Z

webapp/templates/team/submit_modal.html.twig

@@ -80,4 +123,31 @@
            filesSelected.removeClass('d-none');
        });
    </script>
+    <style>
+        .container {


Can you add this to the .ccs files and not directly in the page?

Which existing .css file would be the best to add it to?

webapp/templates/base.html.twig

vmcj

Rebase seemed to have worked.

vmcj · 2024-09-27T06:45:57Z

webapp/src/Controller/Team/SubmissionController.php

+                return $this->redirectToRoute('team_index');
+            }
+        } elseif ($formPaste->isSubmitted() && $formPaste->isValid()) {
+            if ($contest === null) {


A lot of those now get duplicated so maybe merge those where possible? Either by a helper function or using something like:
if ($formUpload->isSubmitted() || $formPaste->isSubmitted()) and do the if/elseif one branch deeper.

I think I've made it much clearer. Is this okay now?

etc/db-config.yaml

vmcj · 2024-09-27T06:48:17Z

webapp/src/Form/Type/SubmitProblemPasteType.php

+            }
+        });
+    }
+}


Missing newline.

vmcj · 2024-09-27T06:48:40Z

webapp/templates/team/submit_modal.html.twig

-                {{ form_start(form) }}
+                {% set active_tab = defaultSubmissionCodeMode == 0 ? 'paste' : 'upload' %}
+
+                <!-- Bootstrap Nav Tabs for Switching -->


I don't think this comment is needed.

as6325400 · 2024-10-01T12:12:47Z

I have mostly finished the modifications as mentioned above. Is there anything else that needs to be changed? Thank you.

vmcj

Looks great already, some small remarks.

I'll wait for someone else to also approve (as is the custom) but this looks good already.

Can you verify again that the unit tests do indeed still pass?

vmcj · 2024-10-03T16:52:12Z

webapp/public/style_domjudge.css

+
+.editor-container:hover {
+    box-shadow: 0 4px 10px rgba(0, 0, 0, 0.2);
+}


The EOF is missing.

I've added the EOF. Thanks for pointing that out!

webapp/templates/team/submit_modal.html.twig

vmcj · 2024-10-03T16:57:42Z

webapp/templates/team/submit_modal.html.twig

+                            <div class="editor-container">
+                                {{ "" | codeEditor(
+                                    "_team_submission_code",
+                                    "c_cpp",


What do you think of moving the language selection above the editor? This way you can also use it here to fix syntax highlighting in the editor (as idea).

I think it's a good idea. When I first implemented it, I wasn't sure how to dynamically adjust the syntax highlighting in the editor by changing the language in the menu. Is that achievable?

Ah, that depends on the twig template.

Let's leave that for a follow up PR in that case.

webapp/templates/team/submit_modal.html.twig

as6325400 · 2024-10-04T01:45:52Z

I found the issue and successfully passed the unit tests.

vmcj · 2024-10-04T16:00:10Z

Can you already squash?

setting paste_code file entry_point add newline and remove comment update paste_code container css site optimize submit logic use cookie to record user submit mode Update webapp/templates/team/submit_modal.html.twig Co-authored-by: MCJ Vasseur <[email protected]> css new line Update webapp/templates/team/submit_modal.html.twig Co-authored-by: MCJ Vasseur <[email protected]> fix Submit page error fix submit page error 2

as6325400 · 2024-10-04T16:12:05Z

Can you already squash?

ok

Can you already squash?

Ok, I already squashed it

meisterT · 2024-10-04T19:12:28Z

webapp/templates/team/submit_modal.html.twig

+                            <div class="modal-footer">
+                                <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>
+                                <button type="submit" class="btn btn-primary">
+                                    <i class="fas fa-paste"></i> Submit Paste


nit: let's call the button just Submit, same with the other button

If both "Upload" and "Paste" are available at the same time, would it be weird or cause confusion if both buttons are labeled "Submit"?

I don't think it is confusing as in both cases you are submitting what you filled in in the form you are currently seeing.

I understand now, I misunderstood what you meant.

meisterT · 2024-10-04T19:25:06Z

webapp/src/Controller/Team/SubmissionController.php

+                    );
+
+                    $files = [$uploadedFile];
+                    $entryPoint = $tempFileName;


The filename as entry point does not work for Kotlin, nor for Java.

See https://github.com/DOMjudge/domjudge/blob/main/webapp/templates/team/partials/submit_scripts.html.twig#L3 or https://github.com/DOMjudge/domjudge/blob/main/submit/submit#L206

Have you considered ignoring the entry in Paste mode, since only one file can be uploaded with Paste?

One example is Java, where the entry point basically specifies the class. If you would make the name of the file clear in the UI (or even an input that can be changed after selecting the problem) we could do the same auto-detection (with possibility to change it) with the entry point as for the uploaded file.

I think it's feasible in terms of implementation, but Paste code is essentially designed for convenience with a single file. If we allow changes to the file name, it somewhat defeats the original purpose of this feature. Alternatively, we could try handling languages where the entry point is not the file name with special treatment? (Actually, this seems similar to the original approach for uploads.)

By the way, I'm currently teaching a data structures class where only C language is allowed, and the usage of Paste is significantly higher than that of upload.

https://github.com/as6325400/domjudge/blob/paste_code/webapp/templates/team/partials/submit_scripts.html.twig#L58-L93

Yes, I followed that approach.

Additionally, I'd like to ask how I can reference code in a comment.

Is there anything else to adjust?

Additionally, I'd like to ask how I can reference code in a comment.

https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-a-permanent-link-to-a-code-snippet#linking-to-code

meisterT · 2024-10-04T19:29:59Z

webapp/src/Controller/Team/SubmissionController.php

+
+                    $uploadedFile = new UploadedFile(
+                        $tempFilePath,
+                        $tempFileName,


I think we should use a simpler name here (and then of course use that as base for the entry point calculation). A nice simple naming scheme could be the shortName of the contestProblem.

I still want the pasted submission to have a special identifier in the filename, to differentiate it from uploads. Is it possible to do this?

What is the reason the way of submitting is important? You could test with adding fileupload- to the name? You would need to test yourself if this still works.

Why is it "fileupload"? I thought it would be something like "Paste."

However, I now think using "shortName" directly is fine too.

If you think the short name as given by the contest problem is ok, let's use that.

Currently, the short name allows various characters. I will still use preg_replace('/[^a-zA-Z0-9_.-]/', '_', $tempFileName); to handle file naming using the short name.

meisterT · 2024-10-04T19:34:20Z

etc/db-config.yaml

@@ -201,6 +201,14 @@
 -   category: Display
    description: Options related to the DOMjudge user interface.
    items:
+        -   name: default_submission_code_mode
+            type: int


Let's make this an array_val, then one can select which ones are enabled. By default let's enable upload for now.

Okay, but in that case, the cookies will also need a major revamp.

You should prevent the array from being empty here. How should I handle it when someone submits the form with an empty array?

Should I use an alert?

I haven’t seen any other similar menus to refer to for this implementation.

Sorry about that.

I would check that the array doesn't get empty and if it does refuse to save or use a danger flash with the message. Alternative is to have a message in the config-check page.

I think implementing the required restriction in the backend can be done in the next PR, as it's more general. Including it in this commit would make things confusing.

can i do it this way?

as6325400 · 2024-10-09T12:23:00Z

I think implementing the required restriction in the backend can be done in the next PR, as it's more general. Including it in this commit would make things confusing.

eldering · 2024-10-09T12:28:02Z

etc/db-config.yaml

+            type: array_val
+            default_value: [upload]
+            public: true
+            description: Select the default submission method for the team


Are these the default or allowed submission methods? Also, it would be good to clarify that these are submission methods from the web interface, as there's also the submit client.

Allowed submission methods, with the default set to upload.

what is submit client?

what is submit client?

This: https://raw.githubusercontent.com/DOMjudge/domjudge/refs/heads/main/submit/submit

as6325400 · 2024-10-18T14:51:53Z

Sorry to bother you, but I was wondering if there are any improvements needed for this?

Once this is merged, I should be able to work on the issue of replacing it with Monaco.

vmcj · 2024-10-18T15:29:14Z

Sorry to bother you, but I was wondering if there are any improvements needed for this?

Once this is merged, I should be able to work on the issue of replacing it with Monaco.

I think you should address @eldering his question, and we should address some of your questions still.

etc/db-config.yaml

vmcj · 2024-10-21T18:42:19Z

webapp/src/Controller/Team/SubmissionController.php

+                        $entryPoint = $formPaste->get('entry_point')->getData() ?: null;
+                        // Check for invalid characters in entry point name
+                        $invalidChars = '/[<>:"\/\\|?*]/';
+                        if(preg_match($invalidChars, $entryPoint)) {


There are some warnings on https://github.com/DOMjudge/domjudge/pull/2720/files which you should still handle.

I still see them:

maybe you have to toggle a to see them.

vmcj · 2024-10-21T18:47:50Z

webapp/src/Controller/Team/SubmissionController.php

+
+                    $uploadedFile = new UploadedFile(
+                        $tempFilePath,
+                        $tempFileName,


What is the reason the way of submitting is important? You could test with adding fileupload- to the name? You would need to test yourself if this still works.

Co-authored-by: MCJ Vasseur <[email protected]>

meisterT · 2024-10-22T18:14:18Z

webapp/src/Controller/Team/SubmissionController.php

+                    );
+                    $tempFileName = preg_replace('/[^a-zA-Z0-9_.-]/', '_', $tempFileName);
+
+                    if ($language->getExtensions()[0] == 'java' || $language->getExtensions()[0] == 'kt') {


Instead of hardcoding Java and Kotlin here, use the $language->getRequireEntryPoint() instead.

Similarly, use $language->getEntryPointDescription() for a description to display.

We probably want to use what we built here so that the entry point works the same regardless of the upload method: https://github.com/DOMjudge/domjudge/blob/main/webapp/src/Form/Type/SubmitProblemType.php#L66

Sorry, I don't understand how to use $language->getRequireEntryPoint(). For example, in the case of Java, when using paste_code, the entry_point might still be no. In this case, how can I use this to determine the file name standard?

I think the remark here is that instead of listing the current languages which need an entrypoint, use that function @meisterT linked and use it from there?

I think the remark here is that instead of listing the current languages which need an entrypoint, use that function @meisterT linked and use it from there?

The function should be checking whether the language in the form requires an entrypoint, which seems to be different from my current situation, right?

Why would it be different? You check for 2 languages here and determine the entrypoint for those. If there are other languages like this we would also want to pick them up. People can add more languages besides those 2.

@vmcj

I'm not sure if my understanding is correct, but are the methods $language->getRequireEntryPoint() and $language->getEntryPointDescription() linked to the part I highlighted in the image?

If so, there might be an issue.

For example, in the case of Java, it can be configured to not require an entry point. In this scenario, under the original upload mode, the uploaded file itself becomes the entry point. However, due to the characteristics of the Java language, the file name must match the main class name.

In the case of the paste code mode, regardless of whether an entry point is required or not, the user must be allowed to specify the main class name. Otherwise, I won’t be able to save the file correctly, which will result in a compile error.

webapp/src/Form/Type/SubmitProblemPasteType.php

Co-authored-by: MCJ Vasseur <[email protected]>

as6325400 mentioned this pull request Sep 23, 2024

add Paste code to submit feature #2712

Closed

as6325400 force-pushed the paste_code branch from 6dcfb05 to 42e4a26 Compare September 25, 2024 07:59

vmcj reviewed Sep 27, 2024

View reviewed changes

webapp/templates/base.html.twig Show resolved Hide resolved

vmcj reviewed Sep 27, 2024

View reviewed changes

vmcj approved these changes Oct 3, 2024

View reviewed changes

vmcj reviewed Oct 3, 2024

View reviewed changes

webapp/templates/team/submit_modal.html.twig Outdated Show resolved Hide resolved

as6325400 requested a review from meisterT October 4, 2024 08:13

as6325400 force-pushed the paste_code branch from 9d134d4 to 62985ba Compare October 4, 2024 16:11

meisterT requested changes Oct 4, 2024

View reviewed changes

as6325400 added 5 commits October 5, 2024 18:04

update button text

56f076e

update submit model display

aa8ed35

fix reset bug

3893628

java, kt entry_point

3376c11

Delete the part that is always true.

bf9d7bc

eldering reviewed Oct 9, 2024

View reviewed changes

as6325400 requested a review from meisterT October 14, 2024 13:12

vmcj reviewed Oct 21, 2024

View reviewed changes

as6325400 and others added 2 commits October 22, 2024 06:57

Update etc/db-config.yaml

add44de

Co-authored-by: MCJ Vasseur <[email protected]>

format

74e36d9

meisterT reviewed Oct 22, 2024

View reviewed changes

change css site and use shortname to name file

dbe2363

nickygerritsen mentioned this pull request Nov 1, 2024

[Feature Request] Submit Code Directly without File #2771

Open

vmcj reviewed Nov 16, 2024

View reviewed changes

webapp/src/Form/Type/SubmitProblemPasteType.php Outdated Show resolved Hide resolved

Update webapp/src/Form/Type/SubmitProblemPasteType.php

723f8d5

Co-authored-by: MCJ Vasseur <[email protected]>

add Paste code to submit feature #2720

Are you sure you want to change the base?

add Paste code to submit feature #2720

Conversation

as6325400 commented Sep 23, 2024

Modify PR merge to main

vmcj commented Sep 25, 2024 • edited Loading

as6325400 commented Sep 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmcj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

as6325400 commented Oct 1, 2024

vmcj left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

as6325400 commented Oct 4, 2024

vmcj commented Oct 4, 2024

as6325400 commented Oct 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

as6325400 commented Oct 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

as6325400 commented Oct 18, 2024

vmcj commented Oct 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meisterT Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmcj commented Sep 25, 2024 •

edited

Loading

vmcj left a comment •

edited

Loading

meisterT Oct 22, 2024 •

edited

Loading