-1
\$\begingroup\$ 
#!/usr/bin/bash

# debug
#export PS4='$LINENO + '
#set -x

# description : #TODO
#
#

# PERSONAL FUNCTION HERE
########################
##
#
#TODO
#
##
########################

# help information
myhelp() {
    #TODO
    echo "usage : mksh [options] {files} "
    echo ''
    echo "      -h,--help check for help"
    echo ''
    echo "      -o,--output output file to path"
}
#

# without argument
no_arg() {
    if [ $# -eq 0 ]; then
        myhelp
    fi
}
#

# DISPOSAL ARGUMENTS
#####################
##
#
cope_args() {
    #TODO
    echo 'disposal args'
}
#
##
#####################

#  cache arguments
arg_menu() {
    #TODO args varities
    help=
    fault=

    while [[ $# -gt 0 ]]; do

        case "$1" in
        -h | --help)
            _LAST_OPT=-h
            help=true
            shift
            ;;
        #TODO needs options
        *)
            echo "invalid : $_LAST_OPT $1"
            fault=true
            break
            ;;
        esac
        
    done

    if [ "$fault" != "true" ]; then
        if [ "$help" == "true" ]; then
            myhelp
        else
            cope_args #TODO need args
        fi
    fi
}
#

# main
main() {
    no_arg "$@"
    arg_menu "$@"
}
#

main "$@"

i want to write a script template , but i didnt have good idea,i just add a parameter interpreter and cut script into many modules,but i dont know how to improve

\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! To help reviewers give you better answers, we need to know what the code is intended to achieve. Please add sufficient context to your question to describe the purpose of the code. We want to know why much more than how. The more you tell us about what your code is for, the easier it will be for reviewers to help you. Also, edit the title to simply summarise the task, rather than your concerns about the code. \$\endgroup\$ Commented Nov 8, 2023 at 15:11

1 Answer 1

Reset to default
1
\$\begingroup\$

Summary: this is not a good Code Review submission.

It is marginally on-topic, due to non-existent / hypothetical code. It is a tool looking for a problem, rather than a concrete solution to a problem.

Is the code bad? No, not really, it is perfectly serviceable. The trouble starts with the problem statement you posed for yourself. You could instead have waited for real problems to arise in your code, then worry about DRYing things up.

Advice to improve:
Recommend you write a couple of scripts that actually do something useful that you care about, then come back and refactor them. You will notice trends and patterns that help you decide what should be extracted as utility code versus what is properly part of each application's code.

It's not clear that "I want to write a template" is the appropriate goal, as that tends to take us down copy-pasta rabbit holes. You might be better off pasting source utility.sh near the top of each application script, and defining utility functions once in there, for better maintainability.

functions

You are using bash functions. Good job, keep it up!

These are good calls:

    no_arg "$@"
    arg_menu "$@"

They are appropriately quoted, and we're obeying the $@ vs. $* distinction to preserve SPACE embedded within an argument. Not sure if you ran the shellcheck linter against this, but you definitely should prior to a commit. (I noticed no issues.)

What's missing is a comment near top of each function explaining what parameters it accepts. Now, these are generic ARGV cracking functions so maybe it's nice enough as-is. But since bash doesn't give us much help in naming the input parameters, it's really on you the author to shoulder that documentation burden at the top. Sometimes we do that as comments, sometimes we will use stylized assignments such as this:

report() {
    out_dir="$1"
    max_size="$2"
    ...
}

or with defaulting:

report() {
    out_dir="${1:-/tmp}"
    max_size="${2:-300}"
    ...
}

It's customary to not verify whether caller passed a spurious $3 arg -- that's just a rough edge of bash functions.

cracking argv

Your while [[ $# -gt 0 ]]; loop seems competent enough. It appears to be much of the motivation for templating in the first place, and I find not a lot of strong motivation there. It should be enough to consult an example each time you encounter such a problem.

Handling fancy options in bash always feels to me like there's too many fiddly bits we must always get right, such as shift; break. For example your --help stanza is nice enough and strictly speaking it doesn't have to break, but stylistically that would be a good pattern for each stanza to finish with.

I see some trivial naming nits. You might rename to my_help for _ underscore-between-words consistency with e.g. no_args. Renaming to the slightly longer but more grammatical cope_with_args wouldn't hurt. Or perhaps parse_args.

language

You chose bash. Ok. It's Turing complete, and much nicer than some alternatives.

Specifically for cracking argv, frankly it's just not great. And it's not like that's the interesting part of writing software. There's actual application logic you should be getting to.

So, you might consider writing in the python scripting language instead:

#! /usr/bin/env python

from pathlib import Path
import os
import typer


def report(out_dir: Path = "/tmp", max_size: int = 200):
    os.chdir(out_dir)
    ...


if __name__ == "__main__":
    typer.run(report)

You get --help "for free":

$ report.py --out-dir /var/tmp --max-size=400
$
$ report.py --help
                                                                                                                                                                        
 Usage: report.py [OPTIONS]                                                                                                                                                  
                                                                                                                                                                        
╭─ Options ──────────────────────────────────────────────────
│ --out-dir         PATH     [default: /tmp]                                                                                                                           │
│ --max-size        INTEGER  [default: 200]                                                                                                                            │
│ --help                     Show this message and exit.                                                                                                               
╰──────────────────────────────────────────────────────────────

ASCII art

My advice on "#######" is: don't do it.

Organizing your code with clear identifiers and sufficient vertical whitespace should suffice. In bash or in any language.

error checking

I notice set -x at top, which can certainly be helpful. Keep in mind that you don't even need to edit source to accomplish that. You can simply invoke as $ bash -x the_script.sh for a debug run.

Some languages started out permissive and then retain that for back-compatibililty, so for example a perl script will often lock things down a bit with use strict;.

It's a good idea to start each bash script with

set -e -u -o pipefail

The -e causes errors (e.g. from ls /non/existant) to exit the script immediately, and option pipefail has the same effect for commands in the middle of a | pipeline.

The -u causes undefined ($foo) variables to error rather than return an empty string.

Clearly these change interpreter behavior. They are designed to help you write in a more conservative dialect, displaying helpful diagnostics when something unexpected may have happened. A single set line at top of utility.sh would suffice, if your application scripts all source that central file.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ thank for your valuable advice,which is very helpful to me as a newbie \$\endgroup\$ Commented Nov 9, 2023 at 6:03

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.