fubark / cyber

Fast and concurrent scripting.
https://cyberscript.dev
MIT License
1.16k stars 38 forks source link

Printer functions seem to duplicate output prefix #89

Open AregevDev opened 4 months ago

AregevDev commented 4 months ago

I am on Windows

Given this C code:

#include <stdio.h>
#include <string.h>

#include "cyber.h"

#define str(x) {x, strlen(x)}

extern "C" {
void printer(CsVM *vm, CsStr str) {
    if (*str.buf != '\n')
        printf("[cyberscript] %.*s\n", (int) str.len, str.buf);
}
}

int main()
{
    CsVM *vm = csCreate();
    csSetPrinter(vm, printer);

    CsValue val;
    CsStr s = str("var a = 2\nprint(a)");
    csEval(vm, s, &val);

    csDestroy(vm);
    return 0;
}

I get the following output

[cyberscript] 2
[cyberscript]
fubark commented 4 months ago

print also adds a new line so it invokes printer twice one for the '2' and another for '\n'. What you could do is check for the newline character and skip it.

You might also want to invoke printf like this: printf("[cyberscript]: %.*s\n", (int)str.len, str.buf); since it doesn't guarantee a zero terminating string.

AregevDev commented 4 months ago

This seems to work

extern "C" {
void printer(CsVM *vm, CsStr str) {
    if (*str.buf != '\n')
        printf("[cyberscript] %.*s\n", (int) str.len, str.buf);
}
}

Maybe cyber should invoke the printer function once. Append the \n to the end of the string instead of calling it twice. I can investigate further and submit a PR for that.

fubark commented 4 months ago

Doing so would either need a fixed buffer where longer strings would get truncated or an extra heap allocation. Neither is a great choice IMO.

I think it's the right abstraction. The printer behaves like a write(ptr, len) interface for stdout similar to the error printer for stderr. I think what would be better is to allow core.print to be replaced from the C-API.

Something like this:

bool onLoad(CLVM* vm, CLModule mod) {
  CLSym sym = clGetSym(mod, "print");
  CLFuncSym func = clGetFirstFunc(sym);
  clSetHostFunc(func, my_print);
}
CLValue my_print(CLVM* vm, const CLValue* args, uint8_t nargs) {
    // str = toString with newline
    clPrint(vm, str);
}

And onLoad would be registered in the module loader, but we'd also need to expose the core modules configuration so that just the onLoad can be replaced.