Skip to content

Adding csplit support for GNU/Linux distros #50

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: master
Choose a base branch
from
Open

Adding csplit support for GNU/Linux distros #50

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bb6c0a
"Added csplit support for multiple distros"
Aug 2, 2020
40ecd14
"Keep the MacOS env check"
Aug 2, 2020
1df7a35
"modified"
Aug 2, 2020
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: 22 additions & 4 deletions createBookFromReadme.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# -------------------- Utility Methods --------------------
# Check for binaries
function checkEnvironment(){
type gcsplit >/dev/null 2>&1 || { echo "Install 'gcsplit' first (e.g. via 'brew install coreutils')." >&2 && exit 1 ; }
type csplit >/dev/null 2>&1 || { echo "Install gcsplit' first (e.g. via 'brew install coreutils')." >&2 && exit 1 ; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't remove the gcsplit line.

Because on macOS gcsplit is used and it cannot be caught in Environment.

What you want to do is check for os type here using an if-else and then execute the check line for that os type.

type mdbook >/dev/null 2>&1 || { echo "Install 'mdbook' first (e.g. via 'cargo install mdbook')." >&2 && exit 1 ; }
}

Expand All @@ -18,15 +18,33 @@ function cleanupBeforeStarting(){

# Splits the Readme.md file based on the header in markdown and creates chapters
# Note:
# Get gcsplit via homebrew on mac: brew install coreutils
# Get csplit via homebrew on mac: brew install coreutils
Copy link
Contributor

@nisrulz nisrulz Jan 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This is again wrong. On macOS you are using the gcsplit still on line 32 in this file. So saying csplit is being installed here is wrong.

You might want to write this:

#   For macOS: Get gcsplit via homebrew, cmd: brew install coreutils
#   For Linux: Get csplit via distro's package manager

function splitIntoChapters(){
OS_NAME=$(uname | tr '[:upper:]' '[:lower:]')

case $OS_NAME in
linux*)
OS_NAME='linux'
csplit --prefix='Chapter_' --suffix-format='%d.md' --elide-empty-files README.md '/^## /' '{*}' -q
;;
darwin*)
OS_NAME='osx'
gcsplit --prefix='Chapter_' --suffix-format='%d.md' --elide-empty-files README.md '/^## /' '{*}' -q
;;
freebsd*)
OS_NAME='freebsd'
csplit --prefix='Chapter_' --suffix-format='%d.md' --elide-empty-files README.md '/^## /' '{*}' -q
;;
*)
OS_NAME=notset
;;
esac
Comment on lines +23 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

I personally donot prefer the switch statements in bash scripts. They are hard to read compared to simpler if-elif statements.

You could have done something like below and executed the specific command:

if [[ "$OSTYPE" == "linux-gnu" ]]; then
    # ...
elif [[ "$OSTYPE" == "darwin"* ]]; then
    # Mac OSX
elif [[ "$OSTYPE" == "cygwin" ]]; then
    # POSIX compatibility layer and Linux environment emulation for Windows
elif [[ "$OSTYPE" == "msys" ]]; then
    # Lightweight shell and GNU utilities compiled for Windows (part of MinGW)
elif [[ "$OSTYPE" == "freebsd"* ]]; then
    #FreeBSD
else
    # Unknown.
fi

}

# Moves generated chapters into src directory
function moveChaptersToSrcDir(){
for f in Chapter_*.md; do
mv $f src/$f
mv "$f" src/"$f"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼

done
}

Expand All @@ -49,7 +67,7 @@ function createSummary(){
# Note:
# Install mdBook as per instructions in their repo https://github.com/rust-lang/mdBook
function buildAndServeBookLocally(){
mdBook build && mdBook serve --open
mdbook build && mdbook serve --open
Comment on lines -52 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

This was fixed in PR #105

}

# -------------------- Steps to create the mdBook version --------------------
Expand Down