Add Linux support, Add local file support#21
Add Linux support, Add local file support#21janniks wants to merge 11 commits intojanniks:masterfrom
Conversation
changed shebang from sh to bash
| @@ -1,12 +1,13 @@ | |||
| #!/bin/sh | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Is there a reason for the switch to bash ?
There was a problem hiding this comment.
The for-loop is using an invalid bracket syntax (((...))) for sh. The bash is a sh extension that understand this format. I'm also not sure if you can use the counter in the for-loop at all (using sh).
On most linuxe the sh is no longer directly installed but a replacement, which is linked to a different shell, so depending on the outcome of this link, the command might work (e.g. on ubuntu sh is linked to dash, which will fail).
If you pipe the script into sh using the curl command, you will also get this error:
Syntax error: Bad for loop variable.
There was a problem hiding this comment.
A more portable solution would be
| #!/bin/bash | |
| #!/usr/bin/env bash |
which works on all systems (including the more obscure ones like NixOS, where /bin/bash does not exist)
scripts/install.sh
Outdated
| PLACEHOLDER_LOGGING_VERBOSE="true" | ||
|
|
||
| GITHUB_SCRIPT_URL="https://raw.githubusercontent.com/janniks/prepare-commit-msg/master/scripts/prepare-commit-msg" | ||
| PLACEHOLDER_NEW_COMMIT_MESSAGE="#{issue_number.upcase}: #{original_commit_message.gsub(/(\s[[:punct:]])+$/, '')}" |
There was a problem hiding this comment.
I like the idea of parameterizing the new commit message. I can also add this to the install script 👍
There was a problem hiding this comment.
Thanks! That was actually the main reason why I forked your project
|
|
||
| printf -- " - Downloading git hook...\n" | ||
| curl -s "$GITHUB_SCRIPT_URL" > "$HOOK_FILE" | ||
| if [ "$should_use_file_from_github_url" = true ] ; then |
There was a problem hiding this comment.
Also a very nice feature 🙌 I might also like to add this to the install script
There was a problem hiding this comment.
Thanks :) I was too lazy to add it to the advanced option and I used it only for testing
| exit | ||
| fi | ||
|
|
|
|
||
| replace_placeholder_with_value () { | ||
| if [ $(uname -s) == "Linux" ]; then | ||
| sed -i "s/${1}/${2}/g" "$HOOK_FILE" |
There was a problem hiding this comment.
Good catch! Is this important/relevant for all Linux?
There was a problem hiding this comment.
I’ve worked on other scripts that use the sed command and it’s been relevant for all Linux user.
Yes, please, of course! And thank you for the script. It's been really helpful so far :) |
c29738d to
68730b2
Compare
|
I updated the script so that it can now use different message styles and different message regexes. Let me know what you think and if you want to merge it :) |
|
Sorry for the delay! I want to merge this 🙈 I'll try in ~ 1-2 weeks, as I am a bit busy currently... |
|
|
||
| ```bash | ||
| sh <(curl -s https://raw.githubusercontent.com/janniks/prepare-commit-msg/master/scripts/install.sh) | ||
| sh <(curl -s https://raw.githubusercontent.com/yrave/prepare-commit-msg/master/scripts/install.sh) |
There was a problem hiding this comment.
| sh <(curl -s https://raw.githubusercontent.com/yrave/prepare-commit-msg/master/scripts/install.sh) | |
| sh <(curl -s https://raw.githubusercontent.com/janniks/prepare-commit-msg/master/scripts/install.sh) |
| @@ -1,12 +1,13 @@ | |||
| #!/bin/sh | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
A more portable solution would be
| #!/bin/bash | |
| #!/usr/bin/env bash |
which works on all systems (including the more obscure ones like NixOS, where /bin/bash does not exist)
| PLACEHOLDER_NEW_COMMIT_MESSAGE="[#{issue_number.upcase}] #{original_commit_message.gsub(/(\s[[:punct:]])+$/, '')}" | ||
| PLACEHOLDER_NEW_COMMIT_MESSAGE_ONE="[#{issue_number.upcase}] #{original_commit_message.gsub(/(\s[[:punct:]])+$/, '')}" | ||
| PLACEHOLDER_NEW_COMMIT_MESSAGE_TWO="#{issue_number.upcase}: #{original_commit_message.gsub(/(\s[[:punct:]])+$/, '')}" | ||
| GITHUB_SCRIPT_URL="https://raw.githubusercontent.com/yrave/prepare-commit-msg/master/scripts/prepare-commit-msg" |
There was a problem hiding this comment.
| GITHUB_SCRIPT_URL="https://raw.githubusercontent.com/yrave/prepare-commit-msg/master/scripts/prepare-commit-msg" | |
| GITHUB_SCRIPT_URL="https://raw.githubusercontent.com/janniks/prepare-commit-msg/master/scripts/prepare-commit-msg" |
|
|
||
| ```bash | ||
| sh <(curl -s https://raw.githubusercontent.com/janniks/prepare-commit-msg/master/scripts/install.sh) | ||
| sh <(curl -s https://raw.githubusercontent.com/yrave/prepare-commit-msg/master/scripts/install.sh) |
There was a problem hiding this comment.
| sh <(curl -s https://raw.githubusercontent.com/yrave/prepare-commit-msg/master/scripts/install.sh) | |
| sh <(curl -s https://raw.githubusercontent.com/janniks/prepare-commit-msg/master/scripts/install.sh) |
If you'll allow it, I'd love to include some of your changes...☺️