Skip to main content
Code Review

Return to Answer

added 2 characters in body
Source Link
J_H
J_H
  • 35k
  • 3
  • 32
  • 129

hardcode

program=kilo
...
echo "$PATH" | awk -v RS=: '{print $1 "/kilo" }'

Please make that awk program look more like this subsequent one:

for file ... '{print $1 "/$program" }' ...

comments

# Check if kilo has been installed

This is accurate.

But the $PATH explanation given in the Review Context would fit nicely with this.

Consider breaking out the for file ... logic as a helper function, so we'll have a nice "if installed then we're cool, else we complain / fixfix".

You might possibly wish to leave e.g. a .bashrc.bak backup file lying around. If nothing else, it lets the user diff to see what changed.

testing

It's possible for someone to try out a shell they seldom use, and therefore lack a dot file for it. But we probably don't care about that corner case.

It appears you didn't test against the fish shell.

elif [ -n "FISH_VERSION" ]; then

Looks like you wanted $ dollar-sign interpolation in that expression.

hardcode

program=kilo
...
echo "$PATH" | awk -v RS=: '{print $1 "/kilo" }'

Please make that awk program look more like this subsequent one:

for file ... '{print $1 "/$program" }' ...

comments

# Check if kilo has been installed

This is accurate.

But the $PATH explanation given in the Review Context would fit nicely with this.

Consider breaking out the for file ... logic as a helper function, so we'll have a nice if installed then we're cool, else we complain / fix.

You might possibly wish to leave e.g. a .bashrc.bak backup file lying around. If nothing else, it lets the user diff to see what changed.

testing

It's possible for someone to try out a shell they seldom use, and therefore lack a dot file for it. But we probably don't care about that corner case.

It appears you didn't test against the fish shell.

elif [ -n "FISH_VERSION" ]; then

Looks like you wanted $ dollar-sign interpolation in that expression.

hardcode

program=kilo
...
echo "$PATH" | awk -v RS=: '{print $1 "/kilo" }'

Please make that awk program look more like this subsequent one:

for file ... '{print $1 "/$program" }' ...

comments

# Check if kilo has been installed

This is accurate.

But the $PATH explanation given in the Review Context would fit nicely with this.

Consider breaking out the for file ... logic as a helper function, so we'll have a nice "if installed then we're cool, else we complain / fix".

You might possibly wish to leave e.g. a .bashrc.bak backup file lying around. If nothing else, it lets the user diff to see what changed.

testing

It's possible for someone to try out a shell they seldom use, and therefore lack a dot file for it. But we probably don't care about that corner case.

It appears you didn't test against the fish shell.

elif [ -n "FISH_VERSION" ]; then

Looks like you wanted $ dollar-sign interpolation in that expression.

Source Link
J_H
J_H
  • 35k
  • 3
  • 32
  • 129

hardcode

program=kilo
...
echo "$PATH" | awk -v RS=: '{print $1 "/kilo" }'

Please make that awk program look more like this subsequent one:

for file ... '{print $1 "/$program" }' ...

comments

# Check if kilo has been installed

This is accurate.

But the $PATH explanation given in the Review Context would fit nicely with this.

Consider breaking out the for file ... logic as a helper function, so we'll have a nice if installed then we're cool, else we complain / fix.

You might possibly wish to leave e.g. a .bashrc.bak backup file lying around. If nothing else, it lets the user diff to see what changed.

testing

It's possible for someone to try out a shell they seldom use, and therefore lack a dot file for it. But we probably don't care about that corner case.

It appears you didn't test against the fish shell.

elif [ -n "FISH_VERSION" ]; then

Looks like you wanted $ dollar-sign interpolation in that expression.