top | item 42037345

(no title)

tripple6 | 1 year ago

Uhm... These scripts seem to be over-engineered if used in scripting, but let me review it. ':)

`git-amend`: A simple `git-commit` alias would support autocomplete and handle all `git-commit` options. Additionally, I would not add the `--no-edit` option by default as editing the commit message would be fine and quickly aborted (say in `nano`), but would introduce another `amend-no-edit` alias as it would be supported in autocomplete anyway.

`git-delete-gone-branches`: This seems to be handy for me. Might be a neat picking, but I would avoid using `awk` in this case in favor of `while IFS=$'\t' read -r ref_name marker ... < <(git for-each-ref --format='%(refname)%09%(upstream:track,nobracket)' ...)`. Additionally, it's a dangerous/destructive operation, it should have the `--force` option. Especially if it deletes a branch if it has commits that are not yet merged.

`git-dir`: The same: an alias is just fine. If used as a user command, sure.

`git-force-pull`: Seems to be fine. I would probably parse the list of tracked branches remotes to be passed to `git-fetch` and then process each with `git-for-each-ref` in a single loop.

`git-forward`: The script seems to execute excessive `git-pull` and `git-fetch` (I would prefer the latter unifying the commands in use), in terms of interacting with the remote repository that would probably need more visible progress verbosity to stderr, and then merely `git-merge` the current branch using the `--ff-only` option, but `git pull --ff-only` is okay too. Since bash supports arrays the script may construct multiple arguments to pass to `git-fetch` so it may fetch more in a single go (but, to be honest, I'm not sure if it would not cause the whole `git-fetch` run to fail if any of its refspec fails for whatever reason).

`git-gc-all`: I would go with an alias as it's clearly a user command, but yes, adding the `--force` would be more tricky perhaps requiring an environment variable like `FORCE` to handle the force flag (i.e. `FORCE= git-gc-all`). Not sure why the script checks whether the command runs in the git repository or a working directory. (Also, it would need the exact path for the script, otherwise it may run in as situation where it would trying to find the `git-in-repo` script in user's current directory.)

`git-in-repo`: Not sure if it's supposed to be used as a user command at all, not a scripting one, but if it's the latter case, git checks the repository directory itself if the given command works with the repository. (N.B. git quirk: deleting a remote repository ref, at least using `git pull -d ...`, requires any local git repo even if it's unrelated to the remote or does not have a remote repository registered in its remotes list...)

`git-is-branch-remote`: I can't pick of a scenario this would be handy for.

`git-is-head-detached`: Not sure if it's supposed to be used in scripts only. From the user perspective, `git-status` or configured `PS1` indicating if `HEAD` is detached would work.

`git-is-worktree-clean`: Another alias candidate if it's supposed to be a user command?

`git-legacy`: To be honest, I didn't figure out how it works and what it's supposed to do. ':)

`git-main-branch`: The script has currently the `origin` remote hardcoded. But I'm not sure if the concept of the main branch exists in git at all.

`git-mode-restore`: This script is crazy. ':) If I understand its purpose, can this be implemented using `git-diff-tree` and `git-update-index`?

`git-root`: Another alias candidate? Is Cygwin pain for certain commands? I also don't know how `cygpath` affects what the Cygwin users do.

`git-xlog`: This seems to be a good candidate as a `git-reflog`/`git-log` builtin, I guess. I have a script, similar to this one, that finds `TODO`-marks introduced (i.e. added lines only) at a specific revision using `git-diff-tree`, but yes it must be combined with `git-rev-list` to work like this one.

General stuff:

* The `USAGE` variable is unnecessarily evaluated everytime any script gets run and does not require to exist: the usage might be defined as a function to be invoked on demand and run external commands like `expand` on demand.

* The scripts may also construct arbitrary options in an array and inject the array to command, hence not requiring command duplicates with sightly another set of options.

* Some of commands from the toolset don't work when launched from the repository directory because they require to be installed first.

* Some of variables are not quoted and may cause unexpected results.

discuss

order

No comments yet.