Skip to content

Add shellcheck support and some fixes to make shell scripts "cleaner" #1074

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

Open
wants to merge 3 commits into
base: release
Choose a base branch
from
Open

Add shellcheck support and some fixes to make shell scripts "cleaner" #1074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ee2a84
Add github action to shellcheck release on push and PRs
yarikoptic Sep 22, 2025
1058c14
[DATALAD RUNCMD] Autopatch using shellcheck (will need fixups)
yarikoptic Sep 22, 2025
bf8649e
Fix shellcheck warnings
yarikoptic Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/shellcheck.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
name: Shellcheck

on:
push:
branches: [release]
pull_request:
branches: [release]

permissions:
contents: read

jobs:
shellcheck:
name: Check shell scripts
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v4
- name: Install dependencies
run: |
sudo apt update && sudo apt install -y shellcheck
- name: shellcheck
run: |
git grep -l '^#\( *shellcheck \|!\(/bin/\|/usr/bin/env \)\(sh\|bash\|dash\|ksh\)\)' | xargs shellcheck
Copy link
Member

Choose a reason for hiding this comment

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

What does *shellcheck do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://www.shellcheck.net/ says in the title "finds bugs in your shell scripts." ;)
Quite often it is so, and most often could be observed if you run any script under path with a space in some folder name . (that is why in our datalad tool testing we even come up with some most obscure path filesystem can tollerate to ensure we always correctly quote etc)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I should have clarified. I know shellcheck (it's great, I use it for all my shell scripts). I was asking about the string "*shellcheck" in the git grep regex.

3 changes: 2 additions & 1 deletion gather-system-info.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
REPO="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"

main() {
cd "${REPO}"
cd "${REPO}" || exit

echo "Please include this information in your bug report on GitHub!"

Expand All @@ -25,6 +25,7 @@ main() {
show_distribution() {
echo -n "Distribution: "
if [ -f /etc/os-release ]; then
# shellcheck disable=SC1091
source /etc/os-release && echo "${NAME}"
fi
}
Expand Down
10 changes: 5 additions & 5 deletions shell.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ export XDG_CONFIG_HOME=$HOME/paperwm/.config
args=()

DISPLAY=$NEW_DISPLAY
eval $(dbus-launch --exit-with-session --sh-syntax)
echo $DBUS_SESSION_BUS_ADDRESS
eval "$(dbus-launch --exit-with-session --sh-syntax)"
echo "$DBUS_SESSION_BUS_ADDRESS"

echo -n $DBUS_SESSION_BUS_ADDRESS \
echo -n "$DBUS_SESSION_BUS_ADDRESS" \
| DISPLAY=$old_display xclip -i -selection clipboard

DISPLAY=$old_display
Expand All @@ -35,13 +35,13 @@ case $1 in
echo "Running X11 Gnome Shell"
Xephyr $NEW_DISPLAY &
DISPLAY=$NEW_DISPLAY
args=--x11
args=(--x11)
Copy link
Member

@smichel17 smichel17 Sep 27, 2025 •
edited by yarikoptic
Loading

Choose a reason for hiding this comment

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

We're not really using the array, so I think we might as well make this x11_arg="--x11" here and gnome-shell "$x11_arg" later

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well, then better be named also x11_arg to be factually correct? since if args then would assume multiple and thus treated accordingly ;-)

;;
esac


dconf reset -f / # Reset settings
dconf write /org/gnome/shell/enabled-extensions "['paperwm@paperwm.github.com']"

gnome-shell $args
gnome-shell "${args[@]}"

Loading