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

Deprecate process shell block #5508

Merged
merged 7 commits into from
Nov 21, 2024
Merged

Deprecate process shell block #5508

merged 7 commits into from
Nov 21, 2024

Conversation

bentsherman
Copy link
Member

Based on some recent internal discussions. The original purpose of the shell block was to help distinguish between Nextflow and Bash variables in the process script. Since the VS Code extension now provides both syntax highlighting and error checking for variable names, the shell block isn't needed as much and can be phased out to promote consistent code.

I didn't bother to add a warning in the code because I will add one in the language server, but we can add one here if you want.

@bentsherman bentsherman requested a review from a team as a code owner November 14, 2024 18:00
Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 4a277a7
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/673e1e9c6c5a1f0008b86c9b

Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
@pditommaso
Copy link
Member

It would make sense to add a deprecation warning (and a warning to disable it)

@christopher-hakkaart
Copy link
Collaborator

Can we clarify the behavior of single (') vs. double quotes (") in this PR as well?

@pbelmann
Copy link

Just my two cents: I use most of the time the terminal to work on Nextflow files, and I think it's still useful to easily distinguish between Nextflow and Bash variables.

@bentsherman
Copy link
Member Author

@pbelmann which terminal editor do you use? the language server is being integrated with other editors here. I believe they already have emacs and neovim

@bentsherman
Copy link
Member Author

@christopher-hakkaart I will address single vs double quotes in the other PR, but I believe shell: is meant to be used always with single quotes so that the dollar sign isn't treated as a Nextflow variable

@pbelmann
Copy link

@pbelmann which terminal editor do you use? the language server is being integrated with other editors here. I believe they already have emacs and neovim

I use Vim most of the time.

@bentsherman
Copy link
Member Author

I'm not sure if vim supports LSP but they just added the nextflow language server to neovim

…sor.groovy

Co-authored-by: Paolo Di Tommaso <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
@pbelmann
Copy link

I'm not sure if vim supports LSP but they just added the nextflow language server to neovim

Thanks for the info. I think I've just been very happy with the fact that you don't need a special editor or editor plugin to work on Nextflow files. I will have a closer look at neovim.

@pditommaso pditommaso merged commit 6f52755 into master Nov 21, 2024
22 checks passed
@pditommaso pditommaso deleted the deprecate-shell-block branch November 21, 2024 06:55
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

Successfully merging this pull request may close these issues.

4 participants