feat: support javascript snippet files #219
Conversation
parsing snippets now first involves attempting to import it as a javascript module, with the default export expected to be the snippets. this enables improved snippet config writing, and could supercede the need for the "variables" setting of the plugin. if importing fails, falls back to existing behavior of parsing as json5 string. security risk: dynamically imported user-provided files may execute malicious code; users should be validate snippet files others share.
… flags in js snippets files, support passing in regexp literals as the trigger. also, generally support regexp flags for regexp snippets (i.e. regexp literals, or snippets with the `r` option). only the following flags are allowed: `i`, `m`, `s`, `u`, `v`. when a snippet with a regexp literal trigger also has flags defined, the flags get merged together.
describe snippet file formats (js, json5); add documentation on added `flags` field; note validity of regex triggers in js snippet files.
|
This is very cool! It also paves the way for allowing users to run code in snippet replacements by letting
I see. I don't think this is much of an issue as it's probably not very likely for ${VISUAL} and other template literals to be used in the same
The idea of prepending
Perhaps we should add a note/warning in the docs regarding this. |
previously, flags were marked as optional field. however, flags are always defined as a string, even if an empty one.
add stricter validation checks, and improve validation check type signature
removes the dependency on json5. if the snippets file isn't a js module with a default export, it now gets treated as a js array via an `export default ` prepended to it, as opposed to a json5 array. json5 is valid js so it should hopefully not break anything
update docs to not mention json5 since it's no longer used. add a warning on readme underneath snippet sharing, to warn about the dangers of running arbitrary javascript code.
typo in code was using unmodified raw options string, so regex literal triggers did not result in `Option.regex` being marked.
ah yes that would be useful; i've made another branch on my fork exploring this (
got it. i will note that the syntax highlighting for regex doesn't exactly play well with obsidian's default theme (see the gif earlier; dark blue on a dark background). seems to be set here:
added a note in the readme on this branch, just underneath the sentence mentioning sharing snippets. also, just realized template literals replacing variables settings might interfere with auto fraction (e.g. allowing spaces between greek letters for detection). i wonder if it might be better to move that into the default snippets (and possibly related utils like |
src/snippets/snippets.ts
Outdated
| "v", | ||
| // "y", // almost certainly undesired behavior | ||
| ]; | ||
| const resolvedFlags = Array.from(new Set(`${override.flags ?? ""}${raw.flags ?? ""}`.split(""))) |
There was a problem hiding this comment.
Is it necessary to include ${raw.flags ?? ""} when override already contains all the flags from raw (because override is defined as {...raw})?
There was a problem hiding this comment.
override.flags was clobbering that it got from raw.flags in the case that trigger was a regexp. i've rewritten the related code in the constructor to make the values easier to trace and remove the use of raw outside of the initial construction of override.
Ah, very nice! I'll have a think about the design and try to get back to you within a week or two.
Ah, I see. Replacing the color with
Hmm, I see how that could be a problem. However, I'm not sure I follow what you mean re: moving things into the default snippets? I agree it'd be nice to expose utils like |
previously was a hardcoded color that was not very readable on dark bg
remove redundant uses of raw snippet, make state of values more explicit.
`RawSnippet` is never actually instantiated, and serves more so as an interface representing the expected shape of a parsed snippet. as such an interface is more appropriate than a `class` for it.
in `ParsedSnippet` constructor, variable was named `override` because it overrode certain fields of `raw` in construction of final parsed snippet. however, `parsed` no longer uses both `raw` and `override`, and so as such `override` more so represents the _resolved_ fields.
hmm, guess i was thinking about the viability of having auto fraction settings deriving from snippets file as well instead of as another setting? something like const GREEK_SYMBOLS = ["alpha", "beta", /* ... */];
// some special export defining what can have a trailing space and still be captured in autofraction
export const AUTOFRACTION_ALLOW_SPACES = [
// this is kinda ugly, because it gets converted to regex it needs double backslashes
...GREEK_SYMBOLS.map(x => "\\\\x"),
"foo",
"bar",
/* ... */
].join("|");i do realize this is a slippery slope to go down though as it could make configuration much harder for those without programming backgrounds. |
I see, thanks for elaborating! It'd certainly be nice to give users more powerful configuration options like this. (It could work well with other options too, like autofraction excluded environments, for example.) As you say, though, we wouldn't want to deter less technical users. One option might be to allow users to configure settings through the plugin settings UI, but also allow power users to override them through snippet files. However, reading settings from two places at once doesn't sound great to me. This PR looks good to me - thank you for your work! |
(submitting this as a pr vs proposal issue because it seems there are a lot of issues backlogged, and makes tentative implementation easy to reference).
Proposal: add support for javascript snippet files.
doing so allows for "native" handling of variables (could largely overshadow the separate variables setting in obsidian) and more programmatic control over snippets.
for a trivial example, i don't like having to write out "trigger", "replacement", etc each time - so i could instead, say, define a function that lets me write shorthand
s({t: ..., r: ..., ...}).js also allows for directly writing regex literals, which allows for more concise, readable snippets (e.g. double-escaping backslashes,
{ trigger: "\\\\command", options: "r" }->{ trigger: /\\command/ })adds support for snippet files to be written as actual js files.
when parsing a snippet file, first tries to parse it as a js esm module, with the snippets array being the default export. that is, the following would be a valid snippets file:
if that fails, falls back to existing parsing mechanism (json5).
there's also some commented out code that could completely remove the need for json5 by prefixing it with
export defaultand parsing it as a module as well (permalink), since json5 is valid es5 js. only notable conflict i've noted between the two formats is that${VISUAL}can't really be replaced by a js variable and so needs to be written as$\{VISUAL}in js files (and, well, just generally, one can execute arbitrary code).dynamically importing user-specified javascript modules notably does provide some security risk; i personally don't think it's too big of an issue, but it is something to keep in mind (and would mean users would need to vet shared snippets more carefully to ensure they are not doing something malicious).
additionally adds support for regex literals in js snippet files, and added a
flagsfield that is applicable to any regex snippet, so e.g. the following is valid:also adds some brief docs explaining the snippets file format, since it seems several people have wondered about it (e.g. #148, #168).