just icon indicating copy to clipboard operation
just copied to clipboard

Add ${just_variable_name} reference in shell script code block

Open linux-china opened this issue 1 year ago • 4 comments

Now I'm developing JetBrains Just plugin, and some developers reported error when editing justfile:

image

It's not Just syntax error, and it's caused by inspection from shell script because {{xxx}} is not legal for shell script in some cases.

Snip20241009_24

Now I must disable language injection for code block, but UX is not good because no more shell error hint for developers when write shell script.

A solution is to use ${just_variable_name} style and make it legal for shell script, still be nice or just to replace placeholder with Just variable. Argc uses this way to deal with variable reference from argc.

{{xxx}} could be kept for expression evaluation, and ${just_variable_name} for variable reference.

linux-china avatar Oct 09 '24 01:10 linux-china

This general approach is not viable in just and anyway doesn't solve the problem.

As you noted, ${just_variable_name} is valid shell syntax. So turning this into just syntax is not backwards-compatible with existing shell script recipes. If someone happens to have a just recipe where a shell variable is named same as a just variable (which I have several such justfile recipes), this syntax addition would change the meaning of the recipe in a way that will break things.

And just recipe bodies are not necessarily shell scripting. They can be any arbitrary language. Even if the ${just_variable_name} syntax were implemented, the same problem would still exist for languages where both {{just_variable_name}} and ${just_variable_name} are invalid syntax.

I'm not familiar with JetBrains at all, so no idea what to suggest for the issue, sorry. In any case, it seems addressing this would need to be done in the JetBrains plugin: a third-party tool erroring on valid just syntax won't have that fixed by just adding a completely new, separate syntax.

laniakea64 avatar Oct 09 '24 03:10 laniakea64

According to https://github.com/search?q=path%3A**%2Fjustfile&type=code&ref=advsearch Most Justfiles use shell as primary language. For shell, I think that it should be allowed developers to choose ${just_variable_name} style according to use cases.

For IDEs, it's some hard to add compatible feature because IDEs uses other tools(ShellCheck, Shfmt, Explainshell) too. Every IDE or editor has different plugin APIs, and sometime it's really hard to implement.

linux-china avatar Oct 09 '24 07:10 linux-china

If just had something like just --show but with interpolations evaluated (like just --dry-run but without printing dependencies), would your JetBrains plugin be able to leverage that functionality for syntax checking?

laniakea64 avatar Oct 09 '24 15:10 laniakea64

@laniakea64 still some hard. IDE/Editor has its incremental AST parser based on Document object from editor. If you change or input some characters in shell script, and it's very expensive to call just command and re-build AST.

linux-china avatar Oct 09 '24 16:10 linux-china

@casey any idea about UX for shell script?

linux-china avatar Oct 16 '24 06:10 linux-china

Actually, this would seem to be a bug in that shell syntax checker. Copying the script shown in the screenshots into a test.sh file, and trying to run it, results in

./test.sh: line 7: {{echo_command}}: command not found

(run outside a git repository - not including the long output from git diff)

With creating an executable named literally {{echo_command}} on that script's $PATH...

echo Echoing "$@"

... then running test.sh produces

Echoing Locally modified files

laniakea64 avatar Oct 16 '24 14:10 laniakea64

I think there's not much that can be done about this. I think one of the benefits of not using the same syntax as the shell, i.e., ${…}, is that you can use ${…} without needing to escape it so it isn't interpreted by just. I think that the long term solution here would be using just syntax instead of shell syntax for code blocks, although that requires just syntax being available in JetBrains.

casey avatar Oct 30 '24 22:10 casey