Skip to content

Send and retrieve language header#27

Open
Blaisorblade wants to merge 7 commits intoracket:masterfrom
ps-tuebingen:upstream/send-language-header
Open

Send and retrieve language header#27
Blaisorblade wants to merge 7 commits intoracket:masterfrom
ps-tuebingen:upstream/send-language-header

Conversation

@Blaisorblade
Copy link
Contributor

Improved version of #22, fixing #17.
In comparison:

  • this sends metadata for the current buffer, not for the last language preference set on any buffer. Thanks to @rfindler for confirming the fix on racket-users.
  • this reloads the metadata on retrieve, reusing DrRacket logic.

Warning: I haven't stress-tested this yet.

Now this is where drracket:tool^ is available.
Now editors->string knows it's only ever called with two arguments.
Fix racket#17 by loading the language metadata from the DrRacket
configuration and prepending it to a clone of the definitions editor
before serializing this clone.

Warning: This hard-codes the module name to `'handin`, which doesn't
sound very good.
This uses many internal APIs, but I don't see how to do that otherwise.
I already simplified significantly the code (as done elsewhere in the
handin-client), removing various calls to internal APIs in the process.
@Blaisorblade
Copy link
Contributor Author

@albertxing: still interested?

@rfindler
Copy link
Member

rfindler commented Oct 4, 2015

Did you try using save-port?

@albertxing
Copy link

@Blaisorblade Only partially. I used a workaround (saving a temporary file and then reading and sending that to the server).

However as @rfindler mentioned save-port would probably be better than save-file

@rfindler
Copy link
Member

rfindler commented Oct 4, 2015

One gotcha to watch out for: the modified field (via set-modified and get-modified) should probably be explicitly tracked and restored so that handin submission doesn't "count" as saving the file.

@Blaisorblade
Copy link
Contributor Author

Did you try using save-port?

TL; DR. I looked into it, but it seemed problematic for many reasons. (1) A simple test confirms save-port doesn't handle metadata, (2) reading the sources suggests it wouldn't work, and (3) the changes save-port does cause would require a nontrivial refactoring of the handin-server.

Below I elaborate in too much detail, because I've tried hard to see whether my analysis was missing something, also because my Racket is still limited; yet all the evidence seems to agree.

  1. To confirm whether metadata would be handled, I hacked a test which wouldn't involve the server. By looking at the output, I could confirm no metadata was visible. Here's the change (on top of my patch):
--- a/handin-client/client-gui.rkt
+++ b/handin-client/client-gui.rkt
@@ -742,6 +742,7 @@
         (write-editor-global-header stream)
         (for ([ed (in-list (list definitions-with-fake-header interactions))]) (send ed write-to-file stream))
         (write-editor-global-footer stream)
+        (send definitions save-port (current-output-port))
         (send base get-bytes)))

     ; Adapted from

You'll find example output at https://gist.github.com/Blaisorblade/99435d25e5a46e0ae645.

  1. Creating metadata requires a filename, which needn't be set when submitting, so it can't be done by save-port, only save-file, in particular through on-save-file and after-load-file. (In fact, I inspected lots of source at first, but this argument seems more compelling).
    Saving:
    https://github.com/racket/drracket/blob/master/drracket/drracket/private/unit.rkt#L567-L585
    Loading: https://github.com/racket/drracket/blob/master/drracket/drracket/private/unit.rkt#L619-L643
    Those hooks also appear to be invoked by save-file and not by save-port (relevant implementation, I think: https://github.com/racket/gui/blob/6ed4157c51201cfea4e9fb4c63e64c44f89ac562/gui-lib/mred/private/wxme/text.rkt#L2775-L2787).

    On the load side, I did try invoking after-load-file, and did get failures because the filename didn't exist. (The culprit was (get-filename), which is also needed by other overrides/augmentations of on-save-file).

  2. save-port totally achieves the "only wxme format when there are images" behavior. Then, it'd be harder to also send the interactions buffer (how do you separate them?). But I don't get why the handin client does that: is that some incomplete feature?
    OTOH, changing this would be too invasive, at least for me: I'd have to understand (and change) input->process->output and submission->bytes 😨 .

@rfindler
Copy link
Member

rfindler commented Oct 5, 2015

Oh, I see. I thought that on/after-save-file were called by save-port.

Thank you for the careful investigation!

I can see how it would be better to copying that bit of code than create a temporary file as part of the handin process (after all, who knows what the state of that disk might be).

@Blaisorblade
Copy link
Contributor Author

Thank you for the careful investigation!

Thank you for accepting it! (In frankness, I've been so thorough because I'm no Racket expert user yet: still need to learn properly about classes and units 😉 ).

@Blaisorblade
Copy link
Contributor Author

Unsubscribing from notifications, please tag me if needed.

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