Skip to content

fix(main): move language server installation to separate shell script #892

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.

Sign up for GitHub

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

Jump to bottom
Merged
RohitKushvaha01 merged 7 commits into from
Oct 21, 2025
Merged

fix(main): move language server installation to separate shell script #892

RohitKushvaha01 merged 7 commits into from
Oct 21, 2025

Conversation

@KonerDev
Copy link
Member

@KonerDev KonerDev commented Oct 20, 2025

Changes

  • Move language server installations to separate shell scripts and improve functionallity
  • Implement Bash language server
  • Fix $PATH variable expanding in .bashrc file

@github-actions
Copy link

github-actions bot commented Oct 20, 2025 •
edited
Loading

PR Summary

Refactored language server installations by moving them into dedicated shell scripts for each LSP, improving modularity and maintainability. Introduced a new Bash language server with ShellCheck integration, enhancing support for Bash scripting. The terminal setup process was also streamlined by extracting common utility functions into a shared utils.sh script and fixing $PATH variable expansion in .bashrc for Node.js installations.

Changes

File Summary
core/main/src/main/assets/terminal/init.sh Refactored init.sh by removing duplicated info, warn, error utility functions and instead sourced them from the new utils.sh script.
core/main/src/main/assets/terminal/lsp/bash.sh New file. Implemented a shell script to install the Bash language server (bash-language-server) and shellcheck. It includes logic to install Node.js LTS and configure npm's global prefix if not already present.
core/main/src/main/assets/terminal/lsp/css.sh New file. Created a shell script to install the CSS language server (vscode-langservers-extracted). It includes checks and installation steps for Node.js LTS and npm global prefix configuration.
core/main/src/main/assets/terminal/lsp/eslint.sh New file. Developed a shell script to install the ESLint language server (vscode-langservers-extracted). It incorporates Node.js LTS installation and npm global prefix setup if necessary.
core/main/src/main/assets/terminal/lsp/html.sh New file. Provided a shell script to install the HTML language server (vscode-langservers-extracted). It includes conditional installation of Node.js LTS and npm global prefix configuration.
core/main/src/main/assets/terminal/lsp/json.sh New file. Introduced a shell script to install the JSON language server (vscode-langservers-extracted). It handles Node.js LTS installation and npm global prefix setup if required.
core/main/src/main/assets/terminal/lsp/markdown.sh New file. Created a shell script to install the Markdown language server (vscode-langservers-extracted). It includes Node.js LTS installation and npm global prefix configuration if needed.
core/main/src/main/assets/terminal/lsp/python.sh New file. Implemented a shell script to install the Python language server (python-lsp-server[all]) using pipx. It ensures pipx is installed and added to the PATH.
core/main/src/main/assets/terminal/lsp/typescript.sh New file. Developed a shell script to install the TypeScript language server (typescript-language-server). It includes Node.js LTS installation and npm global prefix setup if necessary.
core/main/src/main/assets/terminal/setup.sh Refactored setup.sh by removing duplicated info, warn, error utility functions and instead sourced them from the new utils.sh script.
core/main/src/main/assets/terminal/universal_runner.sh Refactored universal_runner.sh by removing duplicated utility functions and sourcing utils.sh. Corrected the PATH variable expansion in ~/.bashrc for Node.js installations to use \$PATH for proper escaping.
core/main/src/main/assets/terminal/utils.sh New file. Created a utility script utils.sh to centralize common shell functions like info, warn, error for colored output and command_exists for checking command availability.
core/main/src/main/java/com/rk/libcommons/editor/ScopeRegistry.kt Updated lspRegistry by adding the new Bash language server, making it available for registration within the editor's scope.
core/main/src/main/java/com/rk/libcommons/editor/lspServers/Bash.kt Implemented the Bash language server class. Defined id, languageName, and supportedExtensions. Updated isInstalled to check for bash-language-server and refactored install to execute the new lsp/bash.sh script. The command method now correctly specifies the Bash LSP execution.
core/main/src/main/java/com/rk/libcommons/editor/lspServers/CSS.kt Refactored the install method to delegate the CSS language server installation to the new lsp/css.sh shell script, simplifying the Kotlin code.
core/main/src/main/java/com/rk/libcommons/editor/lspServers/ESLint.kt Refactored the install method to delegate the ESLint language server installation to the new lsp/eslint.sh shell script, simplifying the Kotlin code.
core/main/src/main/java/com/rk/libcommons/editor/lspServers/HTML.kt Refactored the install method to delegate the HTML language server installation to the new lsp/html.sh shell script, simplifying the Kotlin code.
core/main/src/main/java/com/rk/libcommons/editor/lspServers/JSON.kt Refactored the install method to delegate the JSON language server installation to the new lsp/json.sh shell script, simplifying the Kotlin code.
core/main/src/main/java/com/rk/libcommons/editor/lspServers/Markdown.kt Refactored the install method to delegate the Markdown language server installation to the new lsp/markdown.sh shell script, simplifying the Kotlin code.
core/main/src/main/java/com/rk/libcommons/editor/lspServers/Python.kt Refactored the install method to delegate the Python language server installation to the new lsp/python.sh shell script, simplifying the Kotlin code.
core/main/src/main/java/com/rk/libcommons/editor/lspServers/TypeScript.kt Refactored the install method to delegate the TypeScript language server installation to the new lsp/typescript.sh shell script, simplifying the Kotlin code.
core/main/src/main/java/com/rk/runner/runners/UniversalRunner.kt Refactored the setup logic for universal_runner.sh to utilize the new setupAssetFile utility function, improving code cleanliness and consistency.
core/main/src/main/java/com/rk/xededitor/ui/screens/terminal/TerminalFiles.kt Refactored setupTerminalFiles to use new helper functions setupAssetFile and setupLspFile for managing terminal and LSP-related shell scripts, improving code organization and reducing duplication.

autogenerated by presubmit.ai

github-actions[bot]
github-actions bot reviewed Oct 20, 2025
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Pull request needs attention.

Review Summary

Commits Considered (2)
  • 082c290: feat(main): add Bash language server
  • 01d6ddb: fix(main): move language server installation to separate shell script
Files Processed (19)
  • core/main/src/main/assets/terminal/lsp/bash.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/css.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/eslint.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/html.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/json.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/markdown.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/python.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/typescript.sh (1 hunk)
  • core/main/src/main/assets/terminal/universal_runner.sh (1 hunk)
  • core/main/src/main/java/com/rk/libcommons/editor/ScopeRegistry.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/Bash.kt (1 hunk)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/CSS.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/ESLint.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/HTML.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/JSON.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/Markdown.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/Python.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/TypeScript.kt (2 hunks)
  • core/main/src/main/java/com/rk/xededitor/ui/screens/terminal/TerminalFiles.kt (1 hunk)
Actionable Comments (2)
  • core/main/src/main/assets/terminal/lsp/bash.sh [34-34]

    possible bug: "Fix typo in command existence check."

  • core/main/src/main/assets/terminal/lsp/python.sh [22-22]

    possible bug: "Fix typo in installation message."

Skipped Comments (5)
  • core/main/src/main/assets/terminal/lsp/bash.sh [3-17]

    maintainability: "Refactor common utility functions in shell scripts."

  • core/main/src/main/assets/terminal/lsp/bash.sh [20-20]

    performance: "Optimize package update and upgrade process."

  • core/main/src/main/assets/terminal/lsp/bash.sh [24-24]

    possible issue: "Adjust curl installation placement."

  • core/main/src/main/assets/terminal/lsp/bash.sh [41-41]

    usability: "Review usage of clear command."

  • core/main/src/main/java/com/rk/xededitor/ui/screens/terminal/TerminalFiles.kt [73-73]

    best practice: "Ensure all parent directories are created."

core/main/src/main/assets/terminal/lsp/bash.sh Outdated
export PATH="/home/.npm-global/bin:$PATH"
}

if ! command_exists node || ! command_exist npm; then
Copy link

@github-actions github-actions bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in command_exist npm. It should be command_exists npm to correctly call the defined function.

core/main/src/main/assets/terminal/lsp/python.sh Outdated
info 'Preparing installation...'
apt update && apt upgrade -y

apt 'Installing pipx...'
Copy link

@github-actions github-actions bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line apt 'Installing pipx...' appears to be a typo. apt is a package manager command, not a function for printing messages. It should likely be info 'Installing pipx...' to use the custom info function defined in the script.

github-actions[bot]
github-actions bot reviewed Oct 20, 2025
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM!

Review Summary

Commits Considered (1)
  • 4e207c8: chore(main): refactor terminal file setup
Files Processed (4)
  • core/main/src/main/assets/terminal/lsp/bash.sh (1 hunk)
  • core/main/src/main/java/com/rk/libcommons/editor/ScopeRegistry.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/Bash.kt (1 hunk)
  • core/main/src/main/java/com/rk/xededitor/ui/screens/terminal/TerminalFiles.kt (1 hunk)
Actionable Comments (0)
Skipped Comments (4)
  • core/main/src/main/assets/terminal/lsp/bash.sh [20-20]

    best practice: "Refine package upgrade strategy in installation script."

  • core/main/src/main/assets/terminal/lsp/bash.sh [27-27]

    maintainability: "Verify target directory for global npm packages."

  • core/main/src/main/assets/terminal/lsp/bash.sh [41-41]

    readability: "Avoid clearing terminal output after installation."

  • core/main/src/main/java/com/rk/xededitor/ui/screens/terminal/TerminalFiles.kt [63-63]

    maintainability: "Consider dynamic discovery for LSP files."

github-actions[bot]
github-actions bot reviewed Oct 20, 2025
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)
  • c6dc4ef: fix(main): correct spelling mistakes
Files Processed (8)
  • core/main/src/main/assets/terminal/lsp/bash.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/css.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/eslint.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/html.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/json.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/markdown.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/python.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/typescript.sh (1 hunk)
Actionable Comments (1)
  • core/main/src/main/assets/terminal/lsp/bash.sh [25-25]

    security: "Mitigate security risk of piping curl to bash."

Skipped Comments (4)
  • core/main/src/main/assets/terminal/lsp/bash.sh [22-36]

    maintainability: "Refactor duplicated Node.js installation logic."

  • core/main/src/main/assets/terminal/lsp/bash.sh [20-20]

    best practice: "Avoid full system upgrade in installation scripts."

  • core/main/src/main/assets/terminal/lsp/bash.sh [41-41]

    maintainability: "Review use of clear command."

  • core/main/src/main/assets/terminal/lsp/python.sh [24-24]

    possible bug: "Ensure pipx is in PATH before pipx ensurepath."

@RohitKushvaha01
Copy link
Member

RohitKushvaha01 commented Oct 20, 2025

Btw, in the Bash LSP, you should also install ShellCheck

github-actions[bot]
github-actions bot reviewed Oct 20, 2025
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)
  • 0e19078: fix(main): add shellcheck to bash.sh script and add -y flag to curl installation
Files Processed (7)
  • core/main/src/main/assets/terminal/lsp/bash.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/css.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/eslint.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/html.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/json.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/markdown.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/typescript.sh (1 hunk)
Actionable Comments (1)
  • core/main/src/main/assets/terminal/lsp/bash.sh [3-32]

    maintainability: "Refactor duplicated utility and installation logic into a shared script."

Skipped Comments (1)
  • core/main/src/main/assets/terminal/lsp/bash.sh [20-20]

    best practice: "Review apt upgrade -y for potential unintended side effects."

core/main/src/main/assets/terminal/lsp/bash.sh Outdated
Comment on lines 3 to 32
info() {
printf '\033[34;1m[*] \033[0m%s\n' "$1"
}

warn() {
printf '\033[33;1m[!] \033[0m%s\n' "$1"
}

error() {
printf '\033[31;1m[x] \033[0m%s\n' "$1"
}

command_exists() {
command -v "$1" >/dev/null 2>&1
}

info 'Preparing installation...'
apt update && apt upgrade -y

install_nodejs() {
info "Installing Node.js LTS..."
apt install -y curl
curl -fsSL https://deb.nodesource.com/setup_lts.x | bash -
apt install -y nodejs
mkdir -p /home/.npm-global
npm config set prefix '/home/.npm-global'
grep -qxF "export PATH=\"/home/.npm-global/bin:\$PATH\"" ~/.bashrc || \
echo "export PATH=\"/home/.npm-global/bin:\$PATH\"" >> ~/.bashrc
export PATH="/home/.npm-global/bin:$PATH"
}
Copy link

@github-actions github-actions bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The info, warn, error, command_exists utility functions, and the install_nodejs function are duplicated across all the new language server installation scripts. This leads to significant code duplication and makes maintenance harder. Consider extracting these common functions into a separate shell script (e.g., utils.sh) that can be sourced by all language server scripts. This would improve maintainability and reduce the effort required to update or fix issues in these common components.

Copy link
Member Author

@KonerDev KonerDev Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is important, I still have to do it

@KonerDev
Copy link
Member Author

KonerDev commented Oct 20, 2025

Btw, in the Bash LSP, you should also install ShellCheck

Even with ShellCheck installed, I don't get any diagnostics

@KonerDev KonerDev marked this pull request as draft October 20, 2025 17:43
github-actions[bot]
github-actions bot reviewed Oct 21, 2025
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM!

Review Summary

Commits Considered (7)
  • 5b9dfb1: Merge branch 'main' into fix/lsp-improvements
  • b1e6e83: fix(main): improve terminal file setup and extract util methods into utils.sh
  • 0e19078: fix(main): add shellcheck to bash.sh script and add -y flag to curl installation
  • c6dc4ef: fix(main): correct spelling mistakes
  • 4e207c8: chore(main): refactor terminal file setup
  • 082c290: feat(main): add Bash language server
  • 01d6ddb: fix(main): move language server installation to separate shell script
Files Processed (23)
  • core/main/src/main/assets/terminal/init.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/bash.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/css.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/eslint.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/html.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/json.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/markdown.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/python.sh (1 hunk)
  • core/main/src/main/assets/terminal/lsp/typescript.sh (1 hunk)
  • core/main/src/main/assets/terminal/setup.sh (1 hunk)
  • core/main/src/main/assets/terminal/universal_runner.sh (2 hunks)
  • core/main/src/main/assets/terminal/utils.sh (1 hunk)
  • core/main/src/main/java/com/rk/libcommons/editor/ScopeRegistry.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/Bash.kt (1 hunk)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/CSS.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/ESLint.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/HTML.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/JSON.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/Markdown.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/Python.kt (2 hunks)
  • core/main/src/main/java/com/rk/libcommons/editor/lspServers/TypeScript.kt (2 hunks)
  • core/main/src/main/java/com/rk/runner/runners/UniversalRunner.kt (1 hunk)
  • core/main/src/main/java/com/rk/xededitor/ui/screens/terminal/TerminalFiles.kt (1 hunk)
Actionable Comments (0)
Skipped Comments (4)
  • core/main/src/main/assets/terminal/lsp/bash.sh [6-7]

    maintainability: "Consolidate system update commands."

  • core/main/src/main/assets/terminal/lsp/bash.sh [8-18]

    maintainability: "Extract common Node.js installation logic."

  • core/main/src/main/assets/terminal/lsp/bash.sh [30-30]

    maintainability: "Review use of clear command in installation scripts."

  • core/main/src/main/assets/terminal/lsp/bash.sh [6-6]

    possible issue: "Consider implications of apt upgrade -y in automated installations."

@KonerDev KonerDev marked this pull request as ready for review October 21, 2025 18:29
@RohitKushvaha01 RohitKushvaha01 merged commit ceae874 into Xed-Editor:main Oct 21, 2025
@KonerDev KonerDev deleted the fix/lsp-improvements branch November 1, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@KonerDev @RohitKushvaha01