Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gli_select_specialrequest should parse value.filename to accept responses from GlkOte #35

Open
curiousdannii opened this issue Oct 1, 2020 · 5 comments

Comments

@curiousdannii
Copy link
Contributor

curiousdannii commented Oct 1, 2020

You can't actually directly hook up RemGlk and GlkOte because RemGlk doesn't accept the glk_fileref_create_by_prompt response in the same format as GlkOte sends it: RemGlk expects the filename to be sent in the value property directly, but GlkOte sends value: {filename: 'name'}. I think RemGlk should ideally be able to accept both.

On the other hand, it's one line of code in Emglken...

@erkyrath
Copy link
Owner

erkyrath commented Oct 3, 2020

Didn't I test this?

Ah, no. Maybe I tested it in some long-ago time, but recently I've only tested remglk with the regtest framework. There must be an inconsistency between what regtest sends and what glkote sends here. I'll have to see what happened there.

@curiousdannii
Copy link
Contributor Author

The difference is technically documented:

If the value is missing or null, the user hit Cancel. Otherwise, the value will be a fileref object. This can be used to read or write data to HTML browser storage. (See the documentation in the dialog.js library.)

Note that if GlkOte is connected to a server-side library (such as RemGlk), browser storage is not an appropriate solution. In that case, you would not use dialog.js, but an alternative library (not yet implemented). The user would simply be prompted for a filename, and value would contain this filename as a string.

Using RemGlk on the client side isn't what it's designed for (even though it works really well.) It should be a relatively small amount of code for RemGlk to accept either option, so I thought I'd post this. But then, it's an even smaller amount of code to patch between the formats before RemGlk gets it. Up to you whether you think it's worth it.

@curiousdannii
Copy link
Contributor Author

The same issue occurs in reverse trying to hook up RegTest's RemGlk mode to GlkApi. Again it's easy enough to patch around, but it would be great if there was more consistency.

@erkyrath
Copy link
Owner

erkyrath commented Nov 1, 2020

I've put these changes in on both sides (glkote/glkapi.js and remglk/rgdata.c), but I haven't tested the cross-connecting cases.

The glkapi.js change is the tricky one, since we have to construct a new fileref JS object given just the filename. Or I'm assuming we do, at least! Maybe I'm misunderstanding how the data is being juggled around in your setup.

Give it a try, see if this makes sense.

@curiousdannii
Copy link
Contributor Author

I'll see if I can test some combinations, but maybe not soon. For GlkApi I was using the couple-years-old fork from glkote-term, and it would probably take some work to bring over the newer one. (One day I'll email you about the possibility of working to bring these things together into one repo again.)

But thank you for looking into this!

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

No branches or pull requests

2 participants