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

Support nushell #271

Open
PitiBouchon opened this issue Jun 20, 2024 · 13 comments
Open

Support nushell #271

PitiBouchon opened this issue Jun 20, 2024 · 13 comments

Comments

@PitiBouchon
Copy link

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

@mfontanini
Copy link
Owner

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
Copy link
Author

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

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

@mfontanini
Copy link
Owner

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

@mfontanini
Copy link
Owner

I just added execution support in #274.

@PitiBouchon
Copy link
Author

Thx, I'll check that out

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

@PitiBouchon
Copy link
Author

I did a pr because it was missing something for nushell to work #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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

PitiBouchon commented Jul 2, 2024

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
Copy link
Owner

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 #254 (comment) 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
Copy link
Owner

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

@mfontanini
Copy link
Owner

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

mfontanini added a commit that referenced this issue Jul 13, 2024
Recently it was brought up in
#271 (comment)
that snippet code execution doesn't work in Windows because it used a
bash script to run. This change addresses that and changes the way
executors are defined entirely so that rather than being a bash script
per language, they're defined in a `executors.yaml` file baked into the
`presenterm` binary and can similarly be extended within the config file
placed in `~/.config/presenter/config.yaml`.

This makes defining executors much simpler as you have a single file
with all of them and you don't have to deal with tempdirs and whatnot.
Every time a snippet is executed the following now happens:
* A new tempdir is created.
* The snippet contents are written into `${tempdir}/${filename}` where
`filename` is defined by the snippet execution config.
* A set of commands defined in the execution config are run one by one
using the tempdir as the `cwd`. This means they don't need to deal with
absolute paths or any other annoying detail.
* Optionally a set of environment variables can be set before invoking
all commands.
* The output of all of these commands will be visible in the
presentation.
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