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

A few code tweaks. #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

A few code tweaks. #30

wants to merge 2 commits into from

Conversation

lhunath
Copy link

@lhunath lhunath commented Apr 23, 2019

Changed the script to bash (macOS ships with bash), allows more safe syntax and improves the name pun!

Copy link

@josephholsten josephholsten left a comment

Choose a reason for hiding this comment

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

Ooh, I like these changes!

Copy link

@tagatac tagatac left a comment

Choose a reason for hiding this comment

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

Any chance you could explain the reason for piping to read? What problem were you experiencing that it solves?

)))" | cut -c 2- | awk -v home=$HOME '{print home $0}' | tr '\n' '\0' | xargs -0 -I fname cp fname $BACKUP_DIR/$contactNumber/Attachments
fi

done <(sql "SELECT DISTINCT guid FROM chat;" )
Copy link

Choose a reason for hiding this comment

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

Suggested change
done <(sql "SELECT DISTINCT guid FROM chat;" )
done < <(sql "SELECT DISTINCT guid FROM chat;" )

Without input redirection, this file fails to parse. https://stackoverflow.com/a/26912746

Copy link
Author

Choose a reason for hiding this comment

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

Oops! Yes, absolutely.

@@ -0,0 +1,59 @@
#!/bin/bash
Copy link

Choose a reason for hiding this comment

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

Suggested change
#!/bin/bash
#!/usr/bin/env bash

Don't limit it to the system bash. Mac OS bash is old:

❯❯❯ /bin/bash --version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin19)
Copyright (C) 2007 Free Software Foundation, Inc.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed!

@@ -0,0 +1,59 @@
#!/bin/bash
Copy link

Choose a reason for hiding this comment

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

No need to rename this file. .sh is a fine extension for bash scripts. The advantage of not renaming it is that GitHub will show us your changes inline.

baskup Outdated
sqlite3 ~/Library/Messages/chat.db "$1"
}

for line in $(select_rows "select distinct guid from chat;" ); do
Copy link
Author

Choose a reason for hiding this comment

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

This is replaced by the read because this for relies on "word splitting" while the sqlite3 output is line-based. It's a disagreement between output and parsing that can lead to bugs. Using read, the input is parsed in a line-based manner. IFS is set empty to prevent read's default behaviour of trimming whitespace from the input. It's essentially an idiom, not necessarily tweaked specifically from a requirement of this specific sqlite3's output.

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.

3 participants