mfontanini / presenterm

A markdown terminal slideshow tool
https://mfontanini.github.io/presenterm/
BSD 2-Clause "Simplified" License
1.2k stars 29 forks source link

Support nushell #271

Open PitiBouchon opened 3 months ago

PitiBouchon commented 3 months ago

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

mfontanini commented 3 months ago

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.

PitiBouchon commented 3 months ago

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

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

mfontanini commented 3 months ago

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

mfontanini commented 2 months ago

I just added execution support in #274.

PitiBouchon commented 2 months ago

Thx, I'll check that out

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

PitiBouchon commented 2 months ago

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

mfontanini commented 2 months ago

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...

PitiBouchon commented 2 months ago

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

mfontanini commented 2 months ago

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.

PitiBouchon commented 2 months ago

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

mfontanini commented 2 months ago

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:

mfontanini commented 2 months ago

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

mfontanini commented 2 months ago

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