Ubuntu-fr

kalene

(bash)Demande de conseils pour améliorer Script [Résolu]

Bonsoir j'aimerai vous demander des conseils pour améliorer ce script qui permet de change les propriété d'un fichier (groupe ou utilisateur)

#!/bin/bash
while true; do
    read -rp "Enter the name of the file to change: " FILE
    if [ -z "$FILE" ]; then
        echo "Insert file pls."
    elif [ -e "$FILE" ]; then
        echo "$FILE selected."
        break
    else 
        echo "$FILE file not found."    
    fi
done
function check_if_file_exist(){
    if [ -e "$FILE" ]; then
        return 1
    fi  
}
function check_if_user_exist() {
    LIST_USER=$(cat /etc/passwd | cut -d : -f1)
    for USERS in $LIST_USER; do
        if [ "$CHANGE_USER" = "$USERS" ]; then
            return 1
        fi
    done
}
function change_user(){
    while true; do
        read -rp "Enter the name of the new user: " CHANGE_USER
        if [ -z "$CHANGE_USER" ]; then
            exit 1
        else break
        fi
    done
    while true; do
            check_if_user_exist "$CHANGE_USER"
        if [ $? -eq 1 ]; then
            sudo chown "$CHANGE_USER" "$FILE"
            echo "User $CHANGE_USER is changed"
            break
        else
            echo "User $CHANGE_USER does not exist"
            create_new_user
            break
        fi
    done
}
function create_new_user() {
    while true; do
        read -rp "Do you want create a new user $CHANGE_USER ? (y/n): " yn
        if [[ $yn = [Yy][Ee][Ss] ]] || [[ $yn = [Yy] ]]; then
            check_if_user_exist "$CHANGE_USER"
            if [ $? -eq 0 ]; then 
                sudo adduser "$CHANGE_USER"
                break
            else echo "User $NEW_USER exist"
            fi
        elif [[ $yn = [Nn][Oo] ]] || [[ $yn = [Nn] ]]; then break
        else break
        fi    
    done
   
}
function check_if_group_exist() {
    LIST_GROUP=$(cat /etc/group | cut -d : -f1)
    for GROUP in $LIST_GROUP; do
        if [ "$CHANGE_GROUP" = "$GROUP" ]; then
            return 1
        fi
    done
}
function change_group(){
    while true; do
        read -rp "Enter the name of the new group: " CHANGE_GROUP
        if [ -z "$CHANGE_GROUP" ]; then
            exit 1
            else break
        fi
    done
    while true; do
            check_if_group_exist "$CHANGE_GROUP"
        if [ $? -eq 1 ]; then
            sudo chgrp "$CHANGE_GROUP" "$FILE"
            echo "Group $CHANGE_GROUP is changed"
            break
        else
            echo "Group $CHANGE_GROUP does not exist"
            create_new_group
            break
        fi
    done
}
function create_new_group() {
    while true; do
        read -rp "Do you want create a new group $CHANGE_GROUP ? (y/n): " yn
        if [[ $yn = [Yy][Ee][Ss] ]] || [[ $yn = [Yy] ]]; then
            check_if_group_exist "$CHANGE_GROUP"
            if [ $? -eq 0 ]; then 
                sudo groupadd "$CHANGE_GROUP"
                break 
            else echo "Group $CHANGE_GROUP exist"
            fi
        elif [[ $yn = [Nn][Oo] ]] || [[ $yn = [Nn] ]]; then break 
        else break      
        fi
    done
}
echo "1. Add user"
echo "2. Group user"
while true; do
    read -rp "Do you want add USER or GROUP: " CHOOSE
    if [ -z "$CHOOSE" ]; then
        exit 1
    elif [ "$CHOOSE" = "1" ] || [ "$CHOOSE" = "2" ]; then
        case $CHOOSE in
            "1")
                change_user 
                ;;
            "2")
                change_group 
                ;;
        esac
    fi
done

Merci d'avance

Dernière modification par kalene (Le 01/01/2020, à 21:30)

kamaris

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Déjà, je vois que tu as oublié le conseil de nany et que tu n'as pas passé ton script dans shellcheck.
Donc commence par voir ce qu'il te dit et corrige en fonction

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Si je l'ai fais mais j'arrive pas a les corriger (j'ai pas beaucoup chercher j'avoue) xD mais crois moi si je l'avais pas fait ça serait pire xD mais de se que je vois certaines corrections proposées n'ont aucun sens non ? Genre celle avec cat

kamaris

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Mais si mais si, elle a du sens : par exemple, au lieu de faire

cat /etc/passwd | cut -d : -f1

tu fais

cut -d : -f1 /etc/passwd

Et si cut lisait depuis l'entrée standard, il faudrait faire

cut -d : -f1 < /etc/passwd

Donc dans tous les cas, l'usage de cat est inutile (useless use of cat).

kamaris

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

En local, comme j'exécute shellcheck avec des options un peu plus exigeantes que par défaut, il me dit plus de choses.
Je te le mets ici :

In test.sh line 13 (et 18, 26, 47, 63, 71, 92) :
function check_if_file_exist(){
^-- SC2112: 'function' keyword is non-standard. Delete it.

In test.sh line 55:
            else echo "User $NEW_USER exist"
                            ^-------^ SC2154: NEW_USER is referenced but not assigned.

In test.sh line 114:
        case $CHOOSE in
        ^-- SC2249: Consider adding a default *) case, even if it just exits with error.

For more information:
  https://www.shellcheck.net/wiki/SC2112 -- 'function' keyword is non-standar...
  https://www.shellcheck.net/wiki/SC2154 -- NEW_USER is referenced but not as...
  https://www.shellcheck.net/wiki/SC2249 -- Consider adding a default *) case...

N'oublie pas de consulter les liens vers le wiki de shellcheck, non seulement pour mieux comprendre ce qu'il te dit dans le terminal, mais aussi parce que tu apprendras à coup sûr des choses au passage.

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Ah ok j'avais pas vus le truc comme ça mais c'est logique par contre l'autre j'ai pas compris avec cmd

kamaris

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

As-tu bien lu les pages du wiki de shellcheck comme je te dis en #5 ?
Normalement si tu fais ça, tu dois comprendre, elles sont rédigées de manière assez didactique (bien plus que man bash en tout cas )

Dernière modification par kamaris (Le 31/12/2019, à 01:08)

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

J'ai pas lu le message ta répondu en même temps que j'ai envoyé mon message précédent ! Je vais voir ça

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

J'ai installé le shellcheck en local aussi mais il est pas aussi exigeant que le tien je crois
Voila le script corrigé :

#!/bin/bash
while true; do
    read -rp "Enter the name of the file to change: " FILE
    if [ -z "$FILE" ]; then
        echo "Insert file pls."
    elif [ -e "$FILE" ]; then
        echo "$FILE selected."
        break
    else 
        echo "$FILE file not found."    
    fi
done
function check_if_file_exist(){
    if [ -e "$FILE" ]; then
        return 1
    fi  
}
function check_if_user_exist() {
    LIST_USER=$(cut -d : -f1 /etc/passwd )
    for USERS in $LIST_USER; do
        if [ "$CHANGE_USER" = "$USERS" ]; then
            return 1
        fi
    done
}
function change_user(){
    while true; do
        read -rp "Enter the name of the new user: " CHANGE_USER
        if [ -z "$CHANGE_USER" ]; then
            exit 1
        else break
        fi
    done
    while true; do
            check_if_user_exist "$CHANGE_USER"
        if [ $? -eq 1 ]; then
            sudo chown "$CHANGE_USER" "$FILE"
            echo "User $CHANGE_USER is changed"
            break
        else
            echo "User $CHANGE_USER does not exist"
            create_new_user
            break
        fi
    done
}
function create_new_user() {
    while true; do
        read -rp "Do you want create a new user $CHANGE_USER ? (y/n): " yn
        if [[ $yn = [Yy][Ee][Ss] ]] || [[ $yn = [Yy] ]]; then
            check_if_user_exist "$CHANGE_USER"
            if ! make mytarget; then 
                sudo adduser "$CHANGE_USER"
                break
            else echo "User $CHANGE_USER exist"
            fi
        elif [[ $yn = [Nn][Oo] ]] || [[ $yn = [Nn] ]]; then break
        else break
        fi    
    done
   
}
function check_if_group_exist() {
    LIST_GROUP=$(cut -d : -f1 /etc/group)
    for GROUP in $LIST_GROUP; do
        if [ "$CHANGE_GROUP" = "$GROUP" ]; then
            return 1
        fi
    done
}
function change_group(){
    while true; do
        read -rp "Enter the name of the new group: " CHANGE_GROUP
        if [ -z "$CHANGE_GROUP" ]; then
            exit 1
            else break
        fi
    done
    while true; do
            check_if_group_exist "$CHANGE_GROUP"
        if [ $? -eq 1 ]; then
            sudo chgrp "$CHANGE_GROUP" "$FILE"
            echo "Group $CHANGE_GROUP is changed"
            break
        else
            echo "Group $CHANGE_GROUP does not exist"
            create_new_group
            break
        fi
    done
}
function create_new_group() {
    while true; do
        read -rp "Do you want create a new group $CHANGE_GROUP ? (y/n): " yn
        if [[ $yn = [Yy][Ee][Ss] ]] || [[ $yn = [Yy] ]]; then
            check_if_group_exist "$CHANGE_GROUP"
            if ! make mytarget; then 
                sudo groupadd "$CHANGE_GROUP"
                break 
            else echo "Group $CHANGE_GROUP exist"
            fi
        elif [[ $yn = [Nn][Oo] ]] || [[ $yn = [Nn] ]]; then break 
        else break      
        fi
    done
}
echo "1. Add user"
echo "2. Group user"
while true; do
    read -rp "Do you want add USER or GROUP: " CHOOSE
    if [ -z "$CHOOSE" ]; then
        exit 1
    elif [ "$CHOOSE" = "1" ] || [ "$CHOOSE" = "2" ]; then
        case $CHOOSE in
            "1")
                change_user 
                ;;
            "2")
                change_group 
                ;;
        esac
    fi
done

nany

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Bonjour,

j'exécute shellcheck avec des options un peu plus exigeantes que par défaut

Ça m’intéresse.
Quelles sont ces options ?

[…]
Voila le script corrigé :

[…]
            if ! make mytarget; then 
[…]
            if ! make mytarget; then 
[…]

Huhu ! Ça c’était l’exemple de ShellCheck.
Dans ton cas, il faudrait mettre :

            if 0; then

---

Le mot de passe sudo ne s'affiche pas - Comment bien utiliser les balises BBcode
Retour aux sources.
Urtikan.net - https://youtu.be/fY9NbB8yqFA
LIBRARIES ARE NOT BOOK-STORES!

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Effectivement je trouvais ça bizarre ? merci nany

Watael

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

* les noms des variables doivent ne pas être tout en majuscules; c'est réservé aux variables d'environnement (HOME, SHELL...)
* les fonctions ne devraient pas utiliser des variables globales, mais les paramètres qui leurs sont passés en arguments, et ne pas retourner de variables globales.
* if peut tester directement le code retour d'une commande/fonction :

if commandeOuFonction; then echo "succès commande"; else echo "échec commande"; fi

* ta fonction checkUserExists est mal foutue.
doit-elle réellement traiter une liste de noms d'utilisateurs ?
et si le dernier nom dans la liste USERS (cf. le premier point) existe, la fonction renverra VRAI...
je me contenterais d'un bête

checkUserExists()
{
    local nomUser="$1"
    grep -q "^$nomUser:" /etc/passwd # ou: getent passwd "$nomUSer" &>/dev/null
}

la fonction renverra le code de retour de la dernière commande (grep)

rethink

---

Connected \o/
Welcome to sHell. · eval is evil.

kamaris

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Au niveau de la structure globale du script :

• tu devrais regrouper toutes les fonctions en premier, puis mettre toute la part du script hors fonctions ;

• la logique du script lorsque l'utilisateur ou le groupe n'existent pas encore est bizarre : tu crées cet utilisateur ou ce groupe, et tu t'arrêtes là, au lieu de continuer en modifiant l'utilisateur ou le groupe du fichier (alors même que tu te trouves dans un fonction dont le nom est change_user() ou change_group()) -> il y a probablement un break en trop dans tes boucles while, et d'ailleurs, faut-il utiliser ici une boucle while ?

Quelques remarques plus locales :

• utilise -f plutôt que -e pour tester l'existence d'un fichier, de même que -d plutôt que -e pour les répertoires ;

• la fonction check_if_file_exist() est inutile, et d'ailleurs inutilisée ;

• check_if_user_exist() renvoie 0 ou 1, donc lorsqu'elle doit renvoyer 1, teste directement sa valeur de retour par

  if ! check_if_user_exist; then …

• même remarque pour check_if_group_exist() ;

• pour tes tests sur la réponse saisie par l'utilisateur, tu peux éventuellement faire

  shopt -s nocasematch
  if [[ $yn == @(y|yes) ]]; then …
  if [[ $yn == @(n|no) ]]; then …

• ton test des entrées du script peut être regroupé dans un case-esac global avec un choix par défaut :

  case $CHOOSE in
  '1') change_user ;;
  '2') change_group ;;
  *) exit 1 ;;
  esac

j'exécute shellcheck avec des options un peu plus exigeantes que par défaut

Ça m’intéresse.
Quelles sont ces options ?

J'utilise tout d'abord l'option -s sh de shellcheck, pour lui faire dire des remarques sur les aspects non posix, mais en excluant le code SC2039 qui revient trop souvent sinon en bash.
Ensuite, j'ajoute les codes optionnels suivants : add-default-case, check-unassigned-uppercase, deprecate-which, quote-safe-variables.
Accessoirement, j'indique à shellcheck de suivre le sourçage dans mon répertoire ~/bin.
Au final, je me suis donc mis un alias comme ça dans mon .bashrc :

alias shellcheck='shellcheck -x -s sh -e SC2039 -P ~/bin -o add-default-case,check-unassigned-uppercase,deprecate-which,quote-safe-variables'

nany

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Merci du tuyau.
Malheureusement, je n’ai pas les options -P et -o :

~$ shellcheck -V
ShellCheck - shell script analysis tool
version: 0.4.6
license: GNU General Public License, version 3
website: http://www.shellcheck.net

Dernière modification par nany (Le 31/12/2019, à 14:07)

---

Le mot de passe sudo ne s'affiche pas - Comment bien utiliser les balises BBcode
Retour aux sources.
Urtikan.net - https://youtu.be/fY9NbB8yqFA
LIBRARIES ARE NOT BOOK-STORES!

Watael

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

je n'avais pas vu qu'il y a aussi une fonction if_group_exist, qui fait la même chose que if_user_exist

alors, on peut n'en faire qu'une avec if_user_exist :

# doit être appelée ainsi :
# if_exists database usename
if_exists()
{
   getent "$1" "$2" &>/dev/null
}

mais faire une fonction pour n'appeler qu'une seule commande...
autant n'utiliser que getent !

---

Connected \o/
Welcome to sHell. · eval is evil.

kamaris

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

@nany : ah oui, je suis sous arch :

$ shellcheck -V
ShellCheck - shell script analysis tool
version: 0.7.0
license: GNU General Public License, version 3
website: https://www.shellcheck.net
$ 

Quand j'étais sous ubuntu, je le compilais carrément à la main ce logiciel-là, car de toutes façons ça fait partie des logiciels dont je reçois les notifications de commits.

nany

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Merci kamaris (depuis le temps que je me dis que je dois tester Arch, faudrait peut-être que je m’y mette un jour ).

---

Le mot de passe sudo ne s'affiche pas - Comment bien utiliser les balises BBcode
Retour aux sources.
Urtikan.net - https://youtu.be/fY9NbB8yqFA
LIBRARIES ARE NOT BOOK-STORES!

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Je vais essayer de prendre en compte tout vos avis, mais je suis pas sur de tout comprendre on verra bien sur ce je reviens plus tard vers vous !

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Voile j'ai fais ce que j'ai pus, pour le case kamaris si j'ai fait comme ça c'est pour éviter de stopper le programme si l'utilisateur se trompe dans le tapage du numéro et j'aurais pus mettre les commandes des "checkIf" directement dans les if par exemple "

if getent passwd "$changeUser" &>/dev/null "$@"; then

while true; do
    read -rp "Do you want add USER or GROUP: " choose
    if [ -z "$choose" ]; then
        exit 1
    elif [ "$choose" = "1" ] || [ "$choose" = "2" ]; then
        case $choose in
            "1") changeUser "$@";;
            "2") changeGroup "$@";;
        esac
    fi
done

#!/bin/bash
createNewUser() {
    while true; do
        read -rp "Do you want create a new user $changeUser ? (y/n): " yn
        if [[ $yn == @(y|yes) ]]; then
                sudo adduser "$changeUser"
                echo "User $changeUser create"
                break
        elif [[ $yn == @(n|no) ]]; then break
        else break
        fi    
    done
}
checkIfUserExist() {
getent passwd "$changeUser" &>/dev/null
}
changeUser(){
        read -rp "Enter the name of the new user: " changeUser
        if [ -z "$changeUser" ]; then
            exit 1
        fi
    while true; do
        if checkIfUserExist "$@"; then
            sudo chown "$changeUser" "$file"
            echo "User $changeUser is changed"
            break
        else
            echo "User $changeUser does not exist"
            createNewUser
            break
        fi
    done
}
createNewGroup() {
    while true; do
        read -rp "Do you want create a new group $changeGroup ? (y/n): " yn
        if [[ $yn == @(y|yes) ]]; then
                sudo groupadd "$changeGroup"
                echo "Group $changeGroup create"
                break 
        elif [[ $yn == @(n|no) ]]; then break 
        else break      
        fi
    done
}
checkIfGroupExist() {
getent passwd "$changeGroup" &>/dev/null
}
changeGroup(){
        read -rp "Enter the name of the new group: " changeGroup
        if [ -z "$changeGroup" ]; then
            exit 1
        fi
    while true; do
        if checkIfGroupExist; then
            sudo chgrp "$changeGroup" "$file"
            echo "Group $changeGroup is changed"
            break
        else
            echo "Group $changeGroup does not exist"
            createNewGroup
            break
        fi
    done
}
while true; do
    read -rp "Enter the name of the file to change: " file
    if [ -z "$file" ]; then
        echo "Insert file pls."
    elif [ -f "$file" ]; then
        echo "$file selected."
        break
    else 
        echo "$file file not found."    
    fi
done
echo "1. Add user"
echo "2. Group user"
while true; do
    read -rp "Do you want add USER or GROUP: " choose
    if [ -z "$choose" ]; then
        exit 1
    elif [ "$choose" = "1" ] || [ "$choose" = "2" ]; then
        case $choose in
            "1") changeUser "$@";;
            "2") changeGroup "$@";;
        esac
    fi
done

Watael

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

tu ne peux pas tester si un groupe existe en utilisant la base de données passwd !

et je t'ai dit de ne pas utiliser de paramètres globaux dans tes fonctions mais des paramètres locaux.
il ne faut pas utiliser $changeUser ou $changeGroup, mais passer ces données en arguments :

checkIfExist()
{
    db=$1
    userGroup=$2
    getent "$1" "$2" &>/dev/null
}

if checkIfExist passwd "$changeUser"
then
    ...
fi
if checkIfExist group "$changeGroup"
then
    ...
fi

mais telle quelle la fonction checkIfExist ne sert à rien, fais directement :

if getent group "$changeGroup"

---

Connected \o/
Welcome to sHell. · eval is evil.

kamaris

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Tu passes les arguments d'entrée du script à tes fonctions successives, mais sans jamais les utiliser.
Pourquoi ? Parce que tu as multiplié les fonctions autant de fois qu'il y a d'arguments d'entrée possibles, ce qui est exactement le contraire de ce qu'il faut faire
Une fonction, ça sert à clarifier le code en le factorisant grâce à l'utilisation d'arguments d'entrée.

Tu ne devrais avoir au plus que deux fonctions :

• changeId(), qui prendrait en entrée 1 ou 2 selon qu'on veut changer l'utilisateur ou le groupe ;

• createNewId(), qui prendrait en entrée 1 ou 2 selon qu'on veut créer un nouvel utilisateur ou un nouveau groupe.

À partir de là, le parsing des arguments d'entrée du script serait :

while true; do
	read -rp "Do you want add USER or GROUP: " choose
	case $choose in
	'') exit 1 ;;
	'1'|'2') changeId "$choose" ;;
	*) echo 'Please, choose 1, 2 or leave empty.' ;;
	esac
done

Et là, ça a bien un sens d'appeler une fonction, dont le corps sera celui de changeUser() (ou de changeGroup()) où tu auras paramétrisée la petite nuance user / group.
Et dans cette fonction changeId(), ça aura un sens d'appeler une fonction createNewId(), bâtie de la même façon à partir de createNewUser() ou createNewGroup().
Quant aux fonctions checkIf*Exist(), elles sont inutiles et doivent être supprimées, en utilisant correctement getent (attention à son premier argument d'entrée qui ne doit pas toujours être passwd).

EDIT : il y a de la redite par rapport à ce que vient de dire Watael avant moi, mais bon, c'est pas grave.

Dernière modification par kamaris (Le 31/12/2019, à 22:26)

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Bon voila ce que ca donne ne m'en voulez pas si c'est pas se que vous m'avez dit de faire je comprend pas tout

#!/bin/bash
changeId() {
if [ "$1" = 1 ]; then
        read -rp "Enter the group user name: " changeUser
        if [ -z "$changeUser" ]; then
            exit 1
        fi
    while true; do
        if getent passwd "$changeUser" &>/dev/null; then
            sudo chown "$changeUser" "$file"
            echo "User $changeUser is changed"
            break
        else
            echo "User $changeUser does not exist"
            createNewId "$@"
            break
        fi
    done
fi
if [ "$1" = 2 ]; then
        read -rp "Enter the group name: " changeGroup
        if [ -z "$changeGroup" ]; then
            exit 1
        fi
    while true; do
        if getent group "$changeGroup" &>/dev/null; then
            sudo chgrp "$changeGroup" "$file" 
            echo "Group $changeGroup is changed"
            break
        else
            echo "Group $changeGroup does not exist"
            createNewId "$@"
            break
        fi
    done
fi
}
createNewId() {
if [ "$1" = 1 ]; then
    while true; do
        read -rp "Do you want create a new user $changeUser ? (y/n): " yn
        if [[ $yn == @(y|yes) ]]; then
                sudo adduser "$changeUser"
                echo "User $changeUser create"
                break
        elif [[ $yn == @(n|no) ]]; then break
        else break
        fi    
    done
fi
if [ "$1" = 2 ]; then
    while true; do
        read -rp "Do you want create a new group $changeGroup ? (y/n): " yn
        if [[ $yn == @(y|yes) ]]; then
                sudo groupadd "$changeGroup" 
                echo "Group $changeGroup create"
                break 
        elif [[ $yn == @(n|no) ]]; then break 
        else break      
        fi
    done
fi
}
while true; do
    read -rp "Enter the name of the file to change: " file
    if [ -z "$file" ]; then
        echo "Insert file pls."
    elif [ -f "$file" ]; then
        echo "$file selected."
        break
    else 
        echo "$file file not found."    
    fi
done
echo "1. Change user"
echo "2. Change group"
while true; do
	read -rp "What do you want to change the USER or GROUP: " choose
	case $choose in
	'') exit 1 ;;
	'1'|'2') changeId "$choose" ;;
	*) echo 'Please, choose 1, 2 or leave empty.' ;;
	esac
done

kamaris

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Il faut que tu crées des variables pour paramétriser le code quand il peut l'être, afin le rendre à la fois plus compact et plus lisible : moins de code à lire, plus aéré.
Pour changeId(), ça pourrait donner ça (en intégrant les remarques de mon second point en #13) :

changeId() {
if [ "$1" = 1 ]; then
	id='user'; database='passwd'; cmd='chown'
else
	id='group'; database='group'; cmd='chgrp'
fi

read -rp "Enter the $id name: " idName
if [ -z "$idName" ]; then
	exit 1
fi
if ! getent "$database" "$idName" &>/dev/null; then
	echo "${id^} $idName does not exist"
	createNewId "$1" "$idName"
fi

sudo "$cmd" "$idName" "$file"
echo "${id^} $idName is changed"
}

Ça fait de grands espaces à l'affichage ici, mais c'est parce que j'ai mis des tabulations au lieu des espaces : une fois copiées-collées chez toi, elles prendront la taille que tu auras configurée dans ton éditeur de texte.
Ensuite createNewId() est à remanier de la même façon, en remarquant qu'ici je lui envoie deux paramètres, pour ne pas utiliser de variable globale d'une fonction à l'autre comme te le dit Watael.

Watael

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

if test $1 -eq 1

je préfère test, qui montre que if utilise le code de retour d'une commande (test, ici), plutôt qu'un forme "figée" de groupement comme en awk (if ( foo == "bar" )).
et puisqu'on compare une variable à un nombre, il est préférable d'utiliser un opérateur de comparaison arithmétique (-eq, -gt, -ge...). On aurait pu utiliser un évaluation arithmétique, et écrire

if (( $1 == 1 ))

a priori, rien n'empêche $cmd $idname $file de bien se dérouler, mais rien ne prouve le contraire, le dernier echo pourrait "mentir".

c'est bien de faire des redites : des formulations différentes permettent une meilleure compréhension.
et je suis un peu brouillon dans mes explications.

Dernière modification par Watael (Le 01/01/2020, à 18:21)

---

Connected \o/
Welcome to sHell. · eval is evil.

kalene

Re : (bash)Demande de conseils pour améliorer Script [Résolu]

Merci à vous deux encore. Je commence a bien comprendre maintenant vos corrections, suggestions j'apprends énormément grâce à vous encore merci. Je vais de ce pas changé tout ca, et oui watael tu as tout a fait raison pour les redites.