presenterm icon indicating copy to clipboard operation
presenterm copied to clipboard

Support nushell

Open PitiBouchon opened this issue 1 year ago • 13 comments

It would be great to support nushell for code highlighting and code execution

PitiBouchon avatar Jun 20 '24 12:06 PitiBouchon

presenterm uses bat's syntax files and they don't support nushell so it's not trivial to add support for it here and we'd need to add something separate just for it. If enough people want nushell support or any other language that's not supported by bat, I think it makes sense to add it. But until then I'd like to keep this issue open.

mfontanini avatar Jun 23 '24 22:06 mfontanini

Oh right, I'll check to add nushell syntax highlighting in bat.

But for script execution inside presenterm, it doesn't need bat ?

PitiBouchon avatar Jun 23 '24 23:06 PitiBouchon

Yeah sorry, I forgot about half of the question. Yes, code execution is unrelated and should be straightforward to add :ok_hand:

mfontanini avatar Jun 24 '24 01:06 mfontanini

I just added execution support in #274.

mfontanini avatar Jun 30 '24 22:06 mfontanini

Thx, I'll check that out

It's indeed a mess to add nushell syntax highlighting in bat for now

PitiBouchon avatar Jun 30 '24 22:06 PitiBouchon

I did a pr because it was missing something for nushell to work https://github.com/mfontanini/presenterm/pull/275

Also the way the executor works it spawn a shell script that run the code making nushell only works on linux and not platform like windows.

I'm wondering why always spawning a bash shell to do this instead of spawning the specific shell mentioned (like fish for a fish script etc...)... but supporting nushell on windows would require more investigation and work for now

PitiBouchon avatar Jul 01 '24 20:07 PitiBouchon

For shell scripts in general the code doesn't code through an intermediate bash shell but because of some internal reasons that was not trivial to do for nutshell so I did it this way to make it work and not keep you waiting for to long. I'll do it the proper way soon.

But you do bring up a good point which is that the executor scripts we have now to execute arbitrary code snippets is not Windows friendly...

mfontanini avatar Jul 02 '24 13:07 mfontanini

I think code execution should come with a parameter in the cli and by default it should not execute arbitrary code. Secure by default kind of like the rust philosophy

PitiBouchon avatar Jul 02 '24 13:07 PitiBouchon

You need to explicitly press a key binding now to execute code. Do you think that's not enough? Something I've been thinking is executable code snippets should have some visual indication so you know they're executable.

I do agree that it would be good to be able to enable/disable it, but I feel like disabling by default is a bit harsh.

mfontanini avatar Jul 02 '24 14:07 mfontanini

You can press keys inadvertently, especcialy CTRL+E is not an unusual keybindings and you can have this bindings in other software or press E instead of C if you wanted to CTRL+C.

Yes it may be a bit harsh to require something an option like presenterm -x demo.md to enable code execution. But security should be a priority in general, juste like the rust programming language philosophy is.

I think people should be able to run others / unknown presentation without thinking of priority by default. And requiring -x for instance to run your own markdown file is not much of a pain to do

PitiBouchon avatar Jul 02 '24 14:07 PitiBouchon

press E instead of C if you wanted to CTRL+C

lol not sure I buy this, those keys are very far apart. But I do agree with the general argument and this is actually the reason why I was against this change https://github.com/mfontanini/presenterm/pull/254#issuecomment-2136276413 even though I think that's a good idea. I'll make changes so that:

  • You need to run presenterm -x to enable code execution.
  • And you can override this globally in your config file so you don't have to do this every time. It's up to you: if you only ever run your own presentations you can set this and avoid having to run with -x every time.

mfontanini avatar Jul 03 '24 13:07 mfontanini

This kind of deviated from the original issue but #276 implements what I mentioned in the comment above.

mfontanini avatar Jul 03 '24 13:07 mfontanini

Just as an FYI I changed this in #282 so that this no longer requires an intermediate bash script.

mfontanini avatar Jul 13 '24 00:07 mfontanini

Closing this. You can use a combination of https://github.com/sharkdp/bat#adding-new-syntaxes--language-definitions to use the nushell syntax locally + #388 to invoke bat directly without needing presenterm/bat to support the syntax you want. I will add some docs on this so it's not buried in this ticket.

mfontanini avatar Nov 16 '24 21:11 mfontanini