{"thread":{"id":"58321","subject":"[PATCH 1/2] Make rebase.autostash default","startedAt":"2022-08-18T14:01:09Z","lastAt":"2022-08-18T19:47:52Z","messageCount":9,"participants":["Sergio via GitGitGadget","Sergei Krivonos via GitGitGadget","brian m. carlson","Junio C Hamano","rsbecker@nexbridge.com"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"461422","messageId":"c48fbf984ea42e7c13d56db015dc63c2495f5f5f.1660831231.git.gitgitgadget@gmail.com","threadId":"58321","inReplyTo":"pull.1307.git.git.1660831231.gitgitgadget@gmail.com","subject":"[PATCH 1/2] Make rebase.autostash default","fromName":"Sergio via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T14:00:30Z","receivedAt":"2022-08-18T14:01:09Z","isPatch":true,"sender":{"key":"sergio.callegari@gmail.com","avatar":"https://gravatar.com/avatar/c98f41317e0422c1e630385de0e3970227b8e5ad15f35ba8586066467cc833bc?d=mp&s=160"},"body":"From: Sergio \n\nSigned-off-by: Sergio \n---\n Documentation/config/rebase.txt | 2 +-\n builtin/pull.c | 2 +-\n config.c | 8 ++++++++\n config.h | 8 ++++++++\n 4 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt\nindex f19bd0e0407..bc952327140 100644\n--- a/Documentation/config/rebase.txt\n+++ b/Documentation/config/rebase.txt\n@@ -19,7 +19,7 @@ rebase.autoStash::\n \tsuccessful rebase might result in non-trivial conflicts.\n \tThis option can be overridden by the `--no-autostash` and\n \t`--autostash` options of linkgit:git-rebase[1].\n-\tDefaults to false.\n+\tDefaults to true.\n \n rebase.updateRefs::\n \tIf set to true enable `--update-refs` option by default.\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 403a24d7ca6..333d6a232a7 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -362,7 +362,7 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n \tint status;\n \n \tif (!strcmp(var, \"rebase.autostash\")) {\n-\t\tconfig_autostash = git_config_bool(var, value);\n+\t\tconfig_autostash = git_config_bool_or_default(var, value, 1);\n \t\treturn 0;\n \t} else if (!strcmp(var, \"submodule.recurse\")) {\n \t\trecurse_submodules = git_config_bool(var, value) ?\ndiff --git a/config.c b/config.c\nindex e8ebef77d5c..c4f6da3547e 100644\n--- a/config.c\n+++ b/config.c\n@@ -1437,6 +1437,14 @@ int git_config_bool(const char *name, const char *value)\n \treturn v;\n }\n \n+int git_config_bool_or_default(const char *name, const char *value, int default_value)\n+{\n+\tint v = git_parse_maybe_bool(value);\n+\tif (v < 0)\n+\t\tv = default_value;\n+\treturn v;\n+}\n+\n int git_config_string(const char **dest, const char *var, const char *value)\n {\n \tif (!value)\ndiff --git a/config.h b/config.h\nindex ca994d77147..d236bb0a326 100644\n--- a/config.h\n+++ b/config.h\n@@ -242,6 +242,14 @@ int git_config_bool_or_int(const char *, const char *, int *);\n */\n int git_config_bool(const char *, const char *);\n \n+/**\n+ * Parse a string into a boolean value, respecting keywords like \"true\" and\n+ * \"false\". Integer values are converted into true/false values (when they\n+ * are non-zero or zero, respectively). Other values results in default. If\n+ * parsing is successful, the return value is the result.\n+ */\n+int git_config_bool_or_default(const char *, const char *, int);\n+\n /**\n * Allocates and copies the value string into the `dest` parameter; if no\n * string is given, prints an error message and returns -1.\n-- \ngitgitgadget\n\n"},{"id":"461423","messageId":"pull.1307.git.git.1660831231.gitgitgadget@gmail.com","threadId":"58321","inReplyTo":null,"subject":"[PATCH 0/2] Make rebase.autostash default","fromName":"Sergei Krivonos via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T14:00:29Z","receivedAt":"2022-08-18T14:01:13Z","isPatch":true,"sender":{"key":"name:Sergei Krivonos","avatar":null},"body":"Sergio (2):\n Make rebase.autostash default\n Add Eclipse project settings files to .gitignore\n\n .gitignore | 2 ++\n Documentation/config/rebase.txt | 2 +-\n builtin/pull.c | 2 +-\n config.c | 8 ++++++++\n config.h | 8 ++++++++\n 5 files changed, 20 insertions(+), 2 deletions(-)\n\n\nbase-commit: c50926e1f48891e2671e1830dbcd2912a4563450\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1307%2Fohhmm%2Fmaster-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1307/ohhmm/master-v1\nPull-Request: https://github.com/git/git/pull/1307\n-- \ngitgitgadget\n"},{"id":"461424","messageId":"106a0563cfc29b75dbdbd54ce55140762e133539.1660831231.git.gitgitgadget@gmail.com","threadId":"58321","inReplyTo":"pull.1307.git.git.1660831231.gitgitgadget@gmail.com","subject":"[PATCH 2/2] Add Eclipse project settings files to .gitignore","fromName":"Sergio via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T14:00:31Z","receivedAt":"2022-08-18T14:01:15Z","isPatch":true,"sender":{"key":"sergio.callegari@gmail.com","avatar":"https://gravatar.com/avatar/c98f41317e0422c1e630385de0e3970227b8e5ad15f35ba8586066467cc833bc?d=mp&s=160"},"body":"From: Sergio \n\nSigned-off-by: Sergio \n---\n .gitignore | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/.gitignore b/.gitignore\nindex 42fd7253b44..13755c31caf 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -246,3 +246,5 @@ Release/\n /git.VC.db\n *.dSYM\n /contrib/buildsystems/out\n+/.cproject\n+/.project\n-- \ngitgitgadget\n"},{"id":"461443","messageId":"Yv5u9ApnKDSjSW8O@tapette.crustytoothpaste.net","threadId":"58321","inReplyTo":"c48fbf984ea42e7c13d56db015dc63c2495f5f5f.1660831231.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] Make rebase.autostash default","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2022-08-18T16:55:16Z","receivedAt":"2022-08-18T16:55:53Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2022-08-18 at 14:00:30, Sergio via GitGitGadget wrote:\n> From: Sergio \n> \n> Signed-off-by: Sergio \n\nTypically you'll want to explain in the commit message why this is a\nvaluable change. For example, I don't have this option set and don't use\nit, and I always stash my changes manually before rebasing. You should\ntell me why the user will benefit from this setting defaulting to\nenabled, since I personally don't see a need for it.\n\nYou may also want to discuss why any pitfalls of stash, such as unadded\nchanges being added after a stash pop, are not problematic here and why\nthis behaviour won't be more surprising or annoying to experienced users\nwho are used to seeing an error message instead.\n\nThis isn't to say that the change is bad or we shouldn't accept it, only\nthat I (and others) need help understanding why it's a good change to make.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"461445","messageId":"Yv5wF0DxVe38ygap@tapette.crustytoothpaste.net","threadId":"58321","inReplyTo":"106a0563cfc29b75dbdbd54ce55140762e133539.1660831231.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] Add Eclipse project settings files to .gitignore","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2022-08-18T17:00:07Z","receivedAt":"2022-08-18T17:00:56Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2022-08-18 at 14:00:31, Sergio via GitGitGadget wrote:\n> From: Sergio \n> \n> Signed-off-by: Sergio \n> ---\n> .gitignore | 2 ++\n> 1 file changed, 2 insertions(+)\n> \n> diff --git a/.gitignore b/.gitignore\n> index 42fd7253b44..13755c31caf 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -246,3 +246,5 @@ Release/\n> /git.VC.db\n> *.dSYM\n> /contrib/buildsystems/out\n> +/.cproject\n> +/.project\n\nI have no strong opinion on this change, but typically, to avoid a\nproliferation of patterns with everyone's favourite editor settings, it\ncan be useful if each individual user sets their own editor files in\n~/.config/git/ignore (or core.excludesFile, if you prefer a different\nlocation). For example, I do this with Vim-related files, and it\napplies to all repos on my system, such that other developers don't have\nto care what editor I use.\n\nHowever, Eclipse is a popular editor, so it may be that Junio really\nlikes this change since it will benefit many people.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"461447","messageId":"xmqqy1vlpiqa.fsf@gitster.g","threadId":"58321","inReplyTo":"c48fbf984ea42e7c13d56db015dc63c2495f5f5f.1660831231.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] Make rebase.autostash default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T17:25:01Z","receivedAt":"2022-08-18T17:27:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sergio via GitGitGadget\" writes:\n\n> From: Sergio \n\nNeither the cover letter nor the proposed log message even attempt\nto justify why this is a good change.\n\nProbably that is because it is not justifiable. I do not think it\nis a good idea to change the default, either.\n\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 403a24d7ca6..333d6a232a7 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -362,7 +362,7 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n> \tint status;\n> \n> \tif (!strcmp(var, \"rebase.autostash\")) {\n> -\t\tconfig_autostash = git_config_bool(var, value);\n> +\t\tconfig_autostash = git_config_bool_or_default(var, value, 1);\n\nThis is wrong. What this says is \"if the user has rebase.autostash,\nattempt to interpret its value as a Boolean, and store it in this\nvariable. If the value cannot be read as a Boolean, pretend as if\ntrue was given\".\n\nThat does not set the default to a configuration variable. The\ndefault is the value used when the user does *NOT* specify\nrebase.autostash anywhere, but anything the code does inside the\nblock guarded by that strcmp() cannot affect that case.\n\nIf it were a good idea to make the variable default to true, the\nplace to do so would probably be\n\n diff --git i/builtin/pull.c w/builtin/pull.c\n index 403a24d7ca..0bb8421dfc 100644\n --- i/builtin/pull.c\n +++ w/builtin/pull.c\n @@ -87,7 +87,7 @@ static char *opt_ff;\n static char *opt_verify_signatures;\n static char *opt_verify;\n static int opt_autostash = -1;\n -static int config_autostash;\n +static int config_autostash = 1; /* default to true */\n static int check_trust_level = 1;\n static struct strvec opt_strategies = STRVEC_INIT;\n static struct strvec opt_strategy_opts = STRVEC_INIT;\n\n> diff --git a/config.c b/config.c\n> index e8ebef77d5c..c4f6da3547e 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1437,6 +1437,14 @@ int git_config_bool(const char *name, const char *value)\n> \treturn v;\n> }\n> \n> +int git_config_bool_or_default(const char *name, const char *value, int default_value)\n> +{\n> +\tint v = git_parse_maybe_bool(value);\n> +\tif (v < 0)\n> +\t\tv = default_value;\n> +\treturn v;\n> +}\n\nAnd this is not a useful helper function. At least, this is not\nuseful for this particular case. We have tristate Booleans that\ntake yes/no/auto, and \n\n\tgit_config_bool_or_default(name, value, 2);\n\ncan take \"name.value=auto\" and turn it into 2 (instead of 0=no\n1=yes), but because the helper takes *any* garbage that is not a\nBoolean and gives the same default_value, the value does not have to\nbe \"auto\" here, which makes the helper pretty much useless.\n\nThe patch is incomplete. It only changes \"git pull\" but does not\ndo anything to \"git rebase\".\n"},{"id":"461448","messageId":"xmqqr11dpifv.fsf@gitster.g","threadId":"58321","inReplyTo":"106a0563cfc29b75dbdbd54ce55140762e133539.1660831231.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] Add Eclipse project settings files to .gitignore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T17:31:16Z","receivedAt":"2022-08-18T17:31:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sergio via GitGitGadget\" writes:\n\n> From: Sergio \n>\n> Signed-off-by: Sergio \n> ---\n> .gitignore | 2 ++\n> 1 file changed, 2 insertions(+)\n\nYour interest in the git project is appreciated, but we try to keep\nour .gitignore to the common build artifacts. Things that are\ncreated for those who share a certaion personal preference, like\neditor backup files, are best listed in .git/info/exclude (or\n$HOME/.gitignore). I am an Emacs user but *~ is not in .gitignore\nin the project, for example.\n\nThanks.\n\n\n"},{"id":"461455","messageId":"xmqqa681pga7.fsf@gitster.g","threadId":"58321","inReplyTo":"Yv5wF0DxVe38ygap@tapette.crustytoothpaste.net","subject":"Re: [PATCH 2/2] Add Eclipse project settings files to .gitignore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T18:17:52Z","receivedAt":"2022-08-18T18:17:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> location). For example, I do this with Vim-related files, and it\n> applies to all repos on my system, such that other developers don't have\n> to care what editor I use.\n>\n> However, Eclipse is a popular editor, so it may be that Junio really\n> likes this change since it will benefit many people.\n\nI am all for making new contributor's life better, and in this case,\nNOT adding editor-specific patterns to OUR .gitignore contributes\nbetter for that goal. It will be a shame for us to make a move that\nwill keep our contributors unaware of what they can do with Git, and\nin this case, lack of Eclipse specific patterns did trigger Sergio\nto notice that these are not ignored, and learn that a better place\nto do so is in $HOME/.gitignore, because it will help not only when\nthe contributor works on Git, but when the same contributor works on\nanything using Eclipse. Adding editor-specific patterns ourselves\nrobs such a learning opportunity from new contributors.\n\nThanks.\n"},{"id":"461463","messageId":"032c01d8b33b$5f0253a0$1d06fae0$@nexbridge.com","threadId":"58321","inReplyTo":"xmqqa681pga7.fsf@gitster.g","subject":"RE: [PATCH 2/2] Add Eclipse project settings files to .gitignore","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-08-18T19:47:32Z","receivedAt":"2022-08-18T19:47:52Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On August 18, 2022 2:18 PM, Junio C Hamano wrote:\n>To: brian m. carlson \n>Cc: Sergio via GitGitGadget ; git@vger.kernel.org;\n>Sergei Krivonos \n>Subject: Re: [PATCH 2/2] Add Eclipse project settings files to .gitignore\n>\n>\"brian m. carlson\" writes:\n>\n>> location). For example, I do this with Vim-related files, and it\n>> applies to all repos on my system, such that other developers don't\n>> have to care what editor I use.\n>>\n>> However, Eclipse is a popular editor, so it may be that Junio really\n>> likes this change since it will benefit many people.\n>\n>I am all for making new contributor's life better, and in this case, NOT\nadding\n>editor-specific patterns to OUR .gitignore contributes better for that\ngoal. It will\n>be a shame for us to make a move that will keep our contributors unaware of\n>what they can do with Git, and in this case, lack of Eclipse specific\npatterns did\n>trigger Sergio to notice that these are not ignored, and learn that a\nbetter place to\n>do so is in $HOME/.gitignore, because it will help not only when the\ncontributor\n>works on Git, but when the same contributor works on anything using\nEclipse.\n>Adding editor-specific patterns ourselves robs such a learning opportunity\nfrom\n>new contributors.\n\nThere is a related case to this in ECLIPSE,\nhttps://bugs.eclipse.org/bugs/show_bug.cgi?id=575408, discussing a problem\nwith where ECLIPSE CDT is improperly storing and modifying build settings.\nWhat my project team found is that much of the ECLIPSE settings need to be\npreserved, especially the encodings - .gitignore is not a valid option for\nthese. ECLIPSE has a habit of inheriting container encodings, which is not\nalways correct (we keep our files in UTF-8 not cp1292). Most settings should\nbe retained, but the ones in .settings/language.settings.xml changes each\ntime ECLIPSE restarts or sometimes clones a project. Unfortunately, some of\nthe settings in that file are needed to bootstrap builds for new clones. The\ncase has been open a while with no resolution. Whether a good idea or not,\nwhat our team found an acceptable solution is to use update-index\n--assume-unchanged on that specific file and manage other ECLIPSE artifacts\nin git not excluding them in .gitignore.\n\n--Randall\n\n"}]}