Hot answers tagged

22

You wrote a nice and simple shell. It works for very simple commands but fails on more complex ones (for details, see below). The code can be cleaned up in several places. To make your code nicely readable by others, let an automatic formatter take care of the indentation and spacing. If you have GNU Indent available, use the following command line: indent ...


17

You completely ignored the first points from the review your earlier question, and they still apply here: Whitespace is not a precious resource: your code is far too dense for my taste fewer semicolons, more newlines. try to limit your line length to 90 chars for readability I would go even further, and recommend to stay within 70 chars. ...


17

5 lines ought to be enough for everyone The following has no meaning. Why 5? line1 = input() line2 = input() line3 = input() line4 = input() line5 = input() Instead let the user end: def get_lines(): print("Enter 'END' to end.") lines = [] line = input() while line != 'END': lines.append(line) line = input() return ...


16

Improve your file handling: Don't use a hard coded file name for temporary files. Don't spew temporary files into the current directory. Make sure you clean up temporary files even on error. The first two can be addressed by leveraging mktemp. The last can be addressed with a trap. branchesfile=$(mktemp) trap "{ rm -f $branchesfile; }" EXIT You should ...


14

requests is third-party, so (per the style guide) there should be a blank line in the imports: import os import requests api_repo is constant, so should be API_REPO. There should also be spaces around the = in assignments: API_REPO = 'https://api.github.com/user/repos' Explicit line continuation isn't very Pythonic, especially when the line is short ...


14

DRY - don't repeat yourself "Don't repeat yourself" is a popular programming principle. You could apply it here by not repeating your filename over and over. MY_BRANCHES="5db3bb3c-718a-444c-b1ce-d90a5a0d1cb3.clc" git branch --merged > $MY_BRANCHES nano $MY_BRANCHES # Trim trailing and leading whitespace etc. sed 's/^[ \t]*//;s/[ \t]*$//' < $...


13

Beautiful is better than ugly Your calculator is ugly. @Hosch gave some hints but a much more dramatic redesign is needed. Why must the user say a number that is mapped to a symbol that is mapped to an operation only then applied to the numbers? This is user unfriendliness taken to the extreme. I suggest this very primitive but much more sane ...


11

Concept Review There are a number of purpose built tools to do this job already. The one that comes to mind first is 'shred'. shred is installed by default on all Redhat and Ubuntu systems (in coreutils package on both systems). It is also installed on my RaspberryPi, so, it is ubiquitous. Shred does similar things to what your script does, but it will be ...


10

Do not repeat yourself Make yourself a favor and define a function def clear_screen(): os.system('cls' if os.name == 'nt' else 'clear')


10

Arithmetic operations If you look closely, something is a bit odd here: for (( i = 0; i <= $limit; i+=$1 )); do Ideas? How about: i is written without a dollar, but limit is written with a dollar... In fact you can drop the dollar from limit too, and the expression will be slightly simpler. From $1 you cannot drop it, as the meaning of 1 would be taken ...


9

Looks pretty nice to me. A few remarks: There is no need for the malloc - your structures are small enough so they can live on the stack of main. So you could just do: int main() { char cmd[100]; char* params[10]; .... } then you don't need the free call. It also has the advantage that instead of fgets(cmd, 100, stdin) you can do fgets(cmd, ...


9

do ... done is a compound command; every subcommand shares the file descriptors; so teeing the the loop has the same effect as teeing each subcommand. Two subsequent invocations of echo can be combined together. cat $file is a dreaded UUOC. A basename invocation can be avoided by changing directory to $FOLDER. ls is absolutely unnecessary. The shell already ...


8

I don't really see the point to create with PHP a file that then you have to read in javascript. Can't you just return the content of dummy.html when you make your Ajax request, which will give you a direct access to the data you want in javascript ? Two other (minor) things : I would execute the analysis of the HTML on the PHP side, there are great ...


8

Consider seq Using seq makes your code significantly shorter in those for loops since you can just rely on seq to do your limit checking and the increments. beware that seq is non-standard though. for (( i = 0; i <= $limit; i+=$1 )); do could simply be written as: for i in $(seq 0 $i $limit) do thanks to janos for pointing out the grievous flaws in the ...


8

Overall it's fine. My suggestions would be: Fix the indenting. If you find the comments helpful, great. Otherwise, consider saying "why" and not "what", e.g. # set the var SUDO to "" (blank) -> # This will be used as a prefix to run a command as root You can alternatively consider checking if the command exists by running it: if htop --version > /dev/...


8

In addition to the excellent answer by that other guy: It's a good idea to start the script with a shebang specifying which interpreter to run. In this case, we're not using any Bash features not present in standard POSIX shell, so we can write #!/bin/sh Consider setting the flags -u (using unset variables is an error) and -e (exit when commands fail and ...


7

Overall You're using some bash-specific things ([[ ... && ... ]]), so your shebang line should be #!/bin/bash very good indentation, readable code more quotes: mv "$snippets_dir/$2" "$snippets_dir/$3/$2" list function ls knows how to print to the screen, don't need echo $(ls ...) move function you need echo -e if you want \n to be interpreted as ...


7

User experience -h or --help should print the usage message to standard output, which you do. However, on any unrecognizable command line, you should print the same (or similar) message to standard error and exit with a non-zero status. Similarly, other error messages should go to standard error and result in a non-zero status. Style issues Good job, ...


7

Bug because of subshell You are losing the value of ret_val in a subshell here: ... || (echo "ERROR: Missing required ${each_func}"; ret_val=1) Variable assignments in a subshell are local to the subshell, so the variable in your function won't change. Instead of a subshell, you just want to group these commands using { ...; } like this: ... || { echo "...


7

Your main questions The application is divided into four scripts that have a single task each. This is inspired by how Git is organized. Is this a good way to isolate independent code in bash? If the scripts are short, it's simpler to have all the code in one script, with each main functionality in its own function. If the scripts are longer, and you ...


7

There's not a lot to review here, the only thing that's jumping at me is that this: @Override public void start(Stage primaryStage) { System.out.println("Hello world"); Scanner scanner = new Scanner(System.in); scanner.nextLine(); Platform.exit(); } Should be indented like this: @Override public void start(Stage primaryStage) { ...


7

Here are some general comments to your code, where a lot of these are related to following the guidelines of PEP8: Variable and function names are normally snake_case – In addition the names are normally spelled out, and not abbreviated. I.e. instead of cmd use command, instead of CMDLine use command_line, instead of makeUser use make_user Simplify string ...


7

Avoid a hacker exploit: What happens when the first character read is a null character? if ( fgets(buf, sz, stdin) == NULL ) { return NULL; } buf[strlen(buf) - 1] = '\0'; // same as buf[SIZE_MAX] = '\0'; Instead, use buf[strcspn(buf, "\n")] = '\0'; Expand white-space list to the standard white spaces or use isspace() // char* token = strtok(buf, "...


7

I am assuming you are starting nano so that you may remove possible occurrences of master branch (or some other static branch) from the list. You can do the same in a single line command: git branch --merged | grep -vE "(^\*|master|other|fixed|names)" | xargs git branch -d The ^* check will remove the currently checkedout branch, and you can put in the ...


7

fileExt="*.js" allFiles=$(find ./ -name $fileExt) This is a bug. The wildcard in $fileExt will be expanded by the shell, and cause a syntax error when the current directory has more than one matching file in it: $ touch a.js b.js $ fileExt="*.js" $ find ./ -name $fileExt find: paths must precede expression: `b.js' find: possible unquoted pattern after ...


7

The code is nice and short, and it is easy to read. I'm surprised that it works at all. Sending the out=$(…) command to the background sounds as if the code might continue before curl had the chance to fill the output. Do you get the same results without the &? I'd expect that more names are reported as available then. As long as the & is in the ...


6

Whitespace is not a precious resource: your code is far too dense for my taste fewer semicolons, more newlines. try to limit your line length to 90 chars for readability You rely upon the presence of ~/Music, but never test that it exists. add mkdir -p "$HOME/Music" at the top Variable names: stick with one style, and don't make that style UPPER_CASE (...


6

execvp may fail on a number of reasons, command not found being just one of them (man execve for the list of possible errors. Check errno and call strerr(errno) for a precise error reason and message. Beware that fork may fail too. It is necessary to test pid against -1, and again check errno and call strerr(errno).


Only top voted, non community-wiki answers of a minimum length are eligible