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

Added search and replace for invalid characters in branch name. A '\'… #815

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DarekDan
Copy link

…, '/', and ':', will be replaced with '_'.

@@ -20,6 +20,8 @@ else
if [ "$BRANCH" = "master" ]; then
FILENAME=$ARCHIVE_NAME.$VERSION.zip
else
# govern against invalid path characters
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please follow the indentation of the other code.

Copy link
Author

Choose a reason for hiding this comment

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

The indentation is exactly the same in my IDE, IntelliJ.
19 12 23 09 04 51-git-extras  C__Repos_git-extras  -  _bin_git-archive-file - IntelliJ IDEA (Adm

Copy link
Collaborator

Choose a reason for hiding this comment

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

@DarekDan
Look like your IDE uses 2 spaces to represent a tab, and converts a tab into two space. Would you change the tabs around into 4 spaces?

@@ -20,6 +20,8 @@ else
if [ "$BRANCH" = "master" ]; then
FILENAME=$ARCHIVE_NAME.$VERSION.zip
else
# govern against invalid path characters
BRANCH=$(echo "$BRANCH" | sed -r 's/[\/\\\:]+/_/g')
Copy link
Collaborator

Choose a reason for hiding this comment

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

As g option is specific, is it necessary to use + in the regex?
If + can be omitted, -r can be omitted too.

Copy link
Author

Choose a reason for hiding this comment

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

It does not hurt, and covers all future scenarios.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@DarekDan
Less code, less bugs.
For example, bsd sed doesn't have the -r option. If the extended regex is required, we have to write more code to work around.

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.

2 participants