-
Notifications
You must be signed in to change notification settings - Fork 140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add *_PREVIEW_GIT_OPTS variables #396
base: master
Are you sure you want to change the base?
Conversation
These variables can be used to tune the rendering of the previews The following example allows listing the content of the stash before opening it. FORGIT_STASH_SHOW_PREVIEW_GIT_OPTS="--patch-with-stat --stat-count=10"
This is a WIP code change, I made the changes, but I didn't test them all now. You can still review it now, as the review will be mostly about variables naming and documentation I think So please review |
maybe @carlfriedrich @sandr01d ? |
I will give you a review, but I'm currently very busy and won't make it today. I'll try to find the time tomorrow. |
No rush. I was busy for months. This PR can wait weeks, no worries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your great work @ccoVeille! This is going in the right direction. Most of my comments are me being nitpicky about documentation and a few white space changes that slipped into this PR. I will do a full functional test once the PR has been finalized.
|
||
```shell | ||
export FORGIT_CHECKOUT_BRANCH_BRANCH_GIT_OPTS='--sort=-committerdate' | ||
``` | ||
|
||
- if you want to see a preview of the stashed items in `gss` preview, you could: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- if you want to see a preview of the stashed items in `gss` preview, you could: | |
- if you want to see a overview of the stashed items in `gss` preview, you could: |
I think overview communicates the options from the example better here, since we already use the term preview for the diff that is being displayed.
If you want to customize `git`'s behavior within forgit there is a dedicated variable for each forgit command. | ||
These are passed to the according `git` calls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to customize `git`'s behavior within forgit there is a dedicated variable for each forgit command. | |
These are passed to the according `git` calls. | |
If you want to customize `git`'s behavior within forgit there are dedicated variables for each forgit command. | |
These are passed to the according `git` calls. |
| `gbd` | `FORGIT_BRANCH_DELETE_GIT_OPTS` | | ||
| `gct` | `FORGIT_CHECKOUT_TAG_GIT_OPTS` | | ||
| `gco` | `FORGIT_CHECKOUT_COMMIT_GIT_OPTS`, `FORGIT_CHECKOUT_COMMIT_PREVIEW_GIT_OPTS` | | ||
| `grc` | `FORGIT_REVERT_COMMIT_GIT_OPTS`, `FORGIT_REVERT_PREVIEW_GIT_OPTS` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering whether we should go with FORGIT_REVERT_COMMIT_PREVIEW_GIT_OPTS
here. The name is a bit long, but it would be consistent with the existing FORGIT_REVERT_COMMIT_GIT_OPTS
.
@@ -169,7 +169,7 @@ _forgit_list_files() { | |||
# unquoted. | |||
# uniq is necessary because unmerged files are printed once for each | |||
# merge conflict. | |||
# With the -z flag, git also uses \0 line termination, so we | |||
# With the -z flag, git also uses \0 line termination, so we |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a white space change that should not be here.
|----------|------------------------------------------------------------------------------| | ||
| `ga` | `FORGIT_ADD_GIT_OPTS`, `FORGIT_ADD_PREVIEW_GIT_OPTS` | | ||
| `glo` | `FORGIT_LOG_GIT_OPTS`, `FORGIT_LOG_PREVIEW_GIT_OPTS` | | ||
| `gd` | `FORGIT_DIFF_GIT_OPTS` | | ||
| `grh` | `FORGIT_RESET_HEAD_GIT_OPTS`, `FORGIT_RESET_HEAD_PREVIEW_GIT_OPTS` | | ||
| `gcf` | `FORGIT_CHECKOUT_FILE_GIT_OPTS`, `FORGIT_CHECKOUT_FILE_PREVIEW_GIT_OPTS` | | ||
| `gcb` | `FORGIT_CHECKOUT_BRANCH_GIT_OPTS`, `FORGIT_CHECKOUT_BRANCH_BRANCH_GIT_OPTS` | | ||
| `gbd` | `FORGIT_BRANCH_DELETE_GIT_OPTS` | | ||
| `gct` | `FORGIT_CHECKOUT_TAG_GIT_OPTS` | | ||
| `gco` | `FORGIT_CHECKOUT_COMMIT_GIT_OPTS`, `FORGIT_CHECKOUT_COMMIT_PREVIEW_GIT_OPTS` | | ||
| `grc` | `FORGIT_REVERT_COMMIT_GIT_OPTS`, `FORGIT_REVERT_PREVIEW_GIT_OPTS` | | ||
| `gss` | `FORGIT_STASH_SHOW_GIT_OPTS`, `FORGIT_STASH_SHOW_PREVIEW_GIT_OPTS` | | ||
| `gsp` | `FORGIT_STASH_PUSH_GIT_OPTS`, `FORGIT_STASH_PUSH_PREVIEW_GIT_OPTS` | | ||
| `gclean` | `FORGIT_CLEAN_GIT_OPTS`, `FORGIT_CLEAN_PREVIEW_GIT_OPTS` | | ||
| `grb` | `FORGIT_REBASE_GIT_OPTS`, `FORGIT_FILE_PREVIEW_GIT_OPTS` | | ||
| `gbl` | `FORGIT_BLAME_GIT_OPTS` | | ||
| `gfu` | `FORGIT_FIXUP_GIT_OPTS`, `FORGIT_FILE_PREVIEW_GIT_OPTS` | | ||
| `gcp` | `FORGIT_CHERRY_PICK_GIT_OPTS`, `FORGIT_CHERRY_PICK_PREVIEW_GIT_OPTS` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably note down somewhere which git commands the *_PREVIEW_GIT_OPTS
are used with to let people know which arguments they can use, e.g. it might not be obvious to everyone that FORGIT_LOG_PREVIEW_GIT_OPTS
is used with git show
. Might also make sense to break this out into a separate table if it does not fit into this one.
@@ -531,7 +545,7 @@ _forgit_cherry_pick() { | |||
[[ -z $1 ]] && echo "Please specify target branch" && return 1 | |||
target="$1" | |||
|
|||
# in this function, we do something interesting to maintain proper ordering as it's assumed | |||
# in this function, we do something interesting to maintain proper ordering as it's assumed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...another white space change here
@@ -555,14 +569,16 @@ _forgit_cherry_pick() { | |||
commits+=("$commit") | |||
done < <(echo "$fzf_selection" | sort -n -k 1 | cut -f2 | cut -d' ' -f1 | _forgit_reverse_lines) | |||
[ ${#commits[@]} -eq 0 ] && return 1 | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and here
@@ -858,7 +882,7 @@ _forgit_revert_commit() { | |||
graph=() | |||
[[ $_forgit_log_graph_enable == true ]] && graph=(--graph) | |||
|
|||
# in this function, we do something interesting to maintain proper ordering as it's assumed | |||
# in this function, we do something interesting to maintain proper ordering as it's assumed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and here
@@ -871,7 +895,7 @@ _forgit_revert_commit() { | |||
_forgit_emojify | | |||
nl | | |||
FZF_DEFAULT_OPTS="$opts" fzf | | |||
sort -n -k 1 | | |||
sort -n -k 1 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and here
I'm in holidays, without computer and with limited and sporadic internet connection. I'll give be back in about a week, and will then take a look at your feedbacks |
I'm still busy, I'll be back to normal in the next weeks. |
Check list
Description
These variables can be used to tune the rendering of the previews
The following example allows listing the content of the stash before
opening it.
FORGIT_STASH_SHOW_PREVIEW_GIT_OPTS="--patch-with-stat --stat-count=10"
Type of change
Test environment
Fixes #391