feat: support hidden code lines #254

dmackdev · 2024-05-27T16:12:37Z

This PR adds support for hidden code lines for the languages Rust, Python, Shell and Bash using prefixes at the start of the line, particular to each language. Support for more languages can be added in future.

The hidden code lines will not be visible in snippets in the presentation, but still be evaluated as normal if executed.

See the example presentation for demonstration.

…line, instead of all occurrences

…de shell execution tests

dmackdev · 2024-05-27T16:22:31Z

From #242 (comment):

@dmackdev I think it would be nice if we didn't use /// for every language but this was configurable instead (defaulting to nothing so we don't have to preemptively define it for every language we support highlighting for). /// is valid rust so that would mean I can no longer use doc comments in code snippets, and even worse it would be pretty confusing if you don't know about this, add a /// section and then it doesn't show up in the presentation.

I like the doc tests approach in rust better where # is used to hide lines, see this. Could this be made optional and language-defined so CodeLanguage has some fn hidden_line_prefix(&self) -> Option<&'static str> that can be used to configure this? If you'd like to create the PR and move the conversation there that'd be great.

Totally makes sense @mfontanini - I can make those changes.

So is the thinking that from CodeLanguage::hidden_line_prefix, we return Some("#") for CodeLanguage::Rust, something for CodeLanguage::Bash/Shell, and None for all other languages for now, and we can support for "hidden code lines" for more languages as we go?

dmackdev · 2024-05-27T16:33:09Z

I'd imagine that not all of the supported languages definitely have some equivalent for this prefix in their own docs' syntax. Would we just decide on some sensible prefix in those cases, should the need arise?

mfontanini · 2024-05-27T17:58:03Z

Would we just decide on some sensible prefix in those cases, should the need arise?

Yeah I'd fix those as they come. I imagine in most cases, rust being an exception, the hidden line prefix would be an extension to the language's comment syntax. So e.g. for python/shell script it could be ##, for java /// (but I don't know if this is used for something specifically so I'm unsure). But I wouldn't commit to a default for all languages blindly just in case as it may have a meaning in some of them already.

dmackdev · 2024-05-28T18:41:50Z

src/markdown/elements.rs

+    pub(crate) fn hidden_line_prefix(&self) -> Option<&'static str> {
+        match self {
+            CodeLanguage::Rust => Some("# "),
+            CodeLanguage::Python | CodeLanguage::Shell(_) | CodeLanguage::Bash => Some("## "),


I wonder if there is a better prefix over to use for these languages since a single # denotes a comment in them, so if you wanted to show comments in the snippet, that could potentially be confusing?

How about using ///? I only dislike that in rust because it has a meaning but for the rest of the languages that could work.

That makes sense, can roll with ///.

dmackdev · 2024-05-28T18:42:36Z

examples/hidden_code_lines.md

@@ -0,0 +1,111 @@
+---


This presentation was primarily for demonstration purposes (especially around the line numbering and highlighting). Let me know if you think I should remove/amend it @mfontanini.

This is great but for now I'd like to remove it. I usually write docs about new features along when I'm about to release the next version, and I'll use some of these examples there. I'm planning on, besides having docs for this (and snippet code execution in general, touching the examples/code.md example presentation to include more cases like these ones.

mfontanini · 2024-05-28T23:47:20Z

I'm starting to have second thoughts about this change, specifically from the security angle. With this change you could run a code snippet in a presentation and it could do something like an rm -rf $HOME or other malicious actions without you knowing ahead of time. I think this could make people not want to share presentations / run someone else's presentations. The same goes for the "local executors to the presentation file" that we mentioned in the other issue. If we let the user define this on a per presentation basis, we'd be giving them arbitrary code execution capabilities on whoever runs their presentations' machines.

Thoughts? Am I too paranoid?

dmackdev · 2024-05-29T12:02:15Z

A good point. I think the risk of executing malicious code is present regardless of hidden code lines. All it would take is a user to execute code in a presentation without actually inspecting/understanding: the snippet as it is seen in the presentation, the markdown file, or any local executors. Even without hidden code lines and local executors, there has to be some level of trust that a shared/downloaded presentation does not contain malicious code snippets. Of course, hidden code lines/local executor scripts could elevate this risk - if users rely only on viewing the presentation output and do not inspect the sources.

Indeed, other tools like patat disclose a warning about executing arbitrary code from presentations.

But perhaps we could add some sort of safeguard around this for hidden code lines - maybe a CLI flag --enable-hidden-code-lines, so users have to explicitly take some action to actually hide the prefixed code lines in the presentation. Without the flag, the prefixed code lines would appear in the presentation as normal (without the prefix). Not sure if that would be too much overhead for users, but just a thought.

mfontanini · 2024-05-29T13:14:35Z

there has to be some level of trust that a shared/downloaded presentation does not contain malicious code snippets

Yeah, definitely, but hidden code lines make this a lot more dangerous.

maybe a CLI flag --enable-hidden-code-lines, so users have to explicitly take some action to actually hide the prefixed code lines in the presentation

I agree, if this was implemented we'd need this. However, then it would become annoying for the user that's building a presentation. Now, we could put this as a config option in the config file but this ends up defeating the purpose with foreign presentations. I have mixed feelings about this :/

dmackdev · 2024-06-08T10:19:11Z

Fair enough @mfontanini.
In the absence of any other ideas or a way forward to address the security concerns, feel free to close this PR 🙂

mfontanini

Hey @dmackdev, I'm reviving this! I recently added a configuration/CLI arg that you need to use to explicitly enable snippet code execution in #276. Since that was my main concern and it's now there, I'd like to get this PR in as it adds a lot of value.

I left a couple of comments and it looks like there's some conflicts as well.

mfontanini · 2024-07-07T19:32:55Z

src/markdown/elements.rs

+                    let mut line = format!("{}\n", line);
+                    if line.starts_with(prefix) {
+                        line.replace_range(..prefix.len(), "");
+                    }
+                    line


I think you can replace all of this with something like

let line = line.strip_prefix(prefix).unwrap_or(&line); format!("{line}\n")

Nice one! I have made that change, but also updated it to use fold instead of map as per the Clippy suggestion.

mfontanini · 2024-07-07T23:32:03Z

examples/hidden_code_lines.md

@@ -0,0 +1,111 @@
+---


This is great but for now I'd like to remove it. I usually write docs about new features along when I'm about to release the next version, and I'll use some of these examples there. I'm planning on, besides having docs for this (and snippet code execution in general, touching the examples/code.md example presentation to include more cases like these ones.

mfontanini · 2024-07-07T23:35:32Z

src/markdown/elements.rs

+    pub(crate) fn hidden_line_prefix(&self) -> Option<&'static str> {
+        match self {
+            CodeLanguage::Rust => Some("# "),
+            CodeLanguage::Python | CodeLanguage::Shell(_) | CodeLanguage::Bash => Some("## "),


How about using ///? I only dislike that in rust because it has a meaning but for the rest of the languages that could work.

mfontanini · 2024-07-08T14:29:53Z

Thanks, this is great!

dmackdev added 3 commits May 27, 2024 17:12

feat: add support for hidden code lines

0f4ea5d

fix: replace the hidden code delimiter only at the start of the code …

a77c2e8

…line, instead of all occurrences

test: add Code::visible_lines, Code::executable_contest and hidden co…

f9bdfc0

…de shell execution tests

dmackdev force-pushed the feat/support-hidden-code-lines branch from e9259eb to f9bdfc0 Compare May 27, 2024 16:18

feat: associate hidden line prefix with CodeLanguage

0e91894

dmackdev added 2 commits May 28, 2024 18:16

fix: update prefixes for rust, shell, bash. collate tests

6ab7ae1

feat: update hidden code lines presentation

087bcfb

dmackdev commented May 28, 2024

View reviewed changes

dmackdev marked this pull request as ready for review May 28, 2024 18:42

dmackdev added 2 commits May 28, 2024 19:54

test: add rust executable contents and visible lines tests

e1bfe74

test: reword delimiter to prefix, add interleaved prefix to rust test

ad6e382

mfontanini mentioned this pull request Jul 3, 2024

Support nushell #271

Open

mfontanini reviewed Jul 7, 2024

View reviewed changes

dmackdev added 4 commits July 8, 2024 08:54

Merge branch 'master' into feat/support-hidden-code-lines

7d664d8

use strip prefix and use fold as per clippy suggestion

c6b04f2

change python/shell/bash hidden prefix to ///

aa8cba1

remove hidden code lines presentation

15399b9

dmackdev requested a review from mfontanini July 8, 2024 08:23

mfontanini merged commit 480f947 into mfontanini:master Jul 8, 2024
6 checks passed

dmackdev deleted the feat/support-hidden-code-lines branch July 8, 2024 15:22

BrewTestBot mentioned this pull request Jul 29, 2024

presenterm 0.8.0 Homebrew/homebrew-core#178870

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support hidden code lines #254

feat: support hidden code lines #254

dmackdev commented May 27, 2024 •

edited

Loading

dmackdev commented May 27, 2024 •

edited

Loading

dmackdev commented May 27, 2024

mfontanini commented May 27, 2024

dmackdev May 28, 2024

mfontanini Jul 7, 2024

dmackdev Jul 8, 2024

dmackdev May 28, 2024 •

edited

Loading

mfontanini Jul 7, 2024

dmackdev Jul 8, 2024

mfontanini commented May 28, 2024

dmackdev commented May 29, 2024

mfontanini commented May 29, 2024

dmackdev commented Jun 8, 2024

mfontanini left a comment

mfontanini Jul 7, 2024

dmackdev Jul 8, 2024

mfontanini Jul 7, 2024

mfontanini Jul 7, 2024

mfontanini commented Jul 8, 2024

feat: support hidden code lines #254

feat: support hidden code lines #254

Conversation

dmackdev commented May 27, 2024 • edited Loading

dmackdev commented May 27, 2024 • edited Loading

dmackdev commented May 27, 2024

mfontanini commented May 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmackdev May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfontanini commented May 28, 2024

dmackdev commented May 29, 2024

mfontanini commented May 29, 2024

dmackdev commented Jun 8, 2024

mfontanini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfontanini commented Jul 8, 2024

dmackdev commented May 27, 2024 •

edited

Loading

dmackdev commented May 27, 2024 •

edited

Loading

dmackdev May 28, 2024 •

edited

Loading