{"thread":{"id":"59226","subject":"[PATCH v2 0/3] {apply,alias}: convert pre-processor macros to enums","startedAt":"2023-02-10T17:14:14Z","lastAt":"2023-02-15T08:19:58Z","messageCount":8,"participants":["Vinayak Dev","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"471927","messageId":"20230210171338.81906-1-vinayakdev.sci@gmail.com","threadId":"59226","inReplyTo":null,"subject":"[PATCH v2 0/3] {apply,alias}: convert pre-processor macros to enums","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-02-10T17:13:35Z","receivedAt":"2023-02-10T17:14:14Z","isPatch":true,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"Revert changes to alias.c, and change variable types in apply.c\n\nIt is needed to revert changes to alias.c as the changed #define macros were actually returned values,\nso it is not right to change their types to enums.\n\nAlso, SPLIT_CMDLINE_BAD_ENDING is returned after changing to negative sign,\nbreaking the validity of the conversion to enum. \n\nThe changes to apply.c are actually useful only if the variable types are enum,\nso change the type of variable int patch_method and function argument int side to enums as well.\n\nChanges are in accordance to Eric's review of v1:\nhttps://lore.kernel.org/git/CADE8NappDSaZrMLeqak4is59oL=X1wJOj2eCLLjaCKyrnoK9PQ@mail.gmail.com/T/\n\nVinayak Dev (2):\n Revert added enum to #define\n Change types of int patch_method and int side to enum\n\nVinayak Dev (1):\n Change #define to enum in apply.c and alias.c\n\n alias.c | 1 +\n apply.c | 17 +++++++++++------\n 2 files changed, 12 insertions(+), 6 deletions(-)\n\nRange-diff against v1:\n1: f64b1af2a5 = 1: eaaa09dfb1 Change #define to enum in apply.c and alias.c\n-: ---------- > 2: 86e27e7f64 Revert added enum to #define\n-: ---------- > 3: 17233eb3fe Change types of int patch_method and int side to enum\n-- \n2.39.1\n\n"},{"id":"471928","messageId":"20230210171338.81906-2-vinayakdev.sci@gmail.com","threadId":"59226","inReplyTo":"20230210171338.81906-1-vinayakdev.sci@gmail.com","subject":"[PATCH v2 1/3] {apply,alias}: convert pre-processor macros to enums","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-02-10T17:13:36Z","receivedAt":"2023-02-10T17:14:16Z","isPatch":true,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"From: vinayakdsci \n\nChanges made to alias.c and apply.c in v1, requiring refactoring.\n\nSigned-off-by: Vinayak Dev \n\n---\n alias.c | 12 +++++++++---\n apply.c | 19 +++++++++++++++----\n 2 files changed, 24 insertions(+), 7 deletions(-)\n\ndiff --git a/alias.c b/alias.c\nindex 00abde0817..61ef2c0c54 100644\n--- a/alias.c\n+++ b/alias.c\n@@ -44,9 +44,15 @@ void list_aliases(struct string_list *list)\n \tread_early_config(config_alias_cb, &data);\n }\n \n-#define SPLIT_CMDLINE_BAD_ENDING 1\n-#define SPLIT_CMDLINE_UNCLOSED_QUOTE 2\n-#define SPLIT_CMDLINE_ARGC_OVERFLOW 3\n+/* #define SPLIT_CMDLINE_BAD_ENDING 1 */\n+/* #define SPLIT_CMDLINE_UNCLOSED_QUOTE 2 */\n+/* #define SPLIT_CMDLINE_ARGC_OVERFLOW 3 */\n+enum split_cmdline_error {\n+\tSPLIT_CMDLINE_BAD_ENDING = 1,\n+\tSPLIT_CMDLINE_UNCLOSED_QUOTE,\n+\tSPLIT_CMDLINE_ARGC_OVERFLOW\n+};\n+\n static const char *split_cmdline_errors[] = {\n \tN_(\"cmdline ends with \\\\\"),\n \tN_(\"unclosed quote\"),\ndiff --git a/apply.c b/apply.c\nindex 5eec433583..1e9cf2f4f2 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -205,8 +205,13 @@ struct fragment {\n * or deflated \"literal\".\n */\n #define binary_patch_method leading\n-#define BINARY_DELTA_DEFLATED\t1\n-#define BINARY_LITERAL_DEFLATED 2\n+/* #define BINARY_DELTA_DEFLATED 1 */\n+/* #define BINARY_LITERAL_DEFLATED 2 */\n+\n+enum binary_type_deflated {\n+\tBINARY_DELTA_DEFLATED = 1,\n+\tBINARY_LITERAL_DEFLATED\n+};\n \n static void free_fragment_list(struct fragment *list)\n {\n@@ -918,8 +923,14 @@ static int gitdiff_hdrend(struct gitdiff_data *state UNUSED,\n * their names against any previous information, just\n * to make sure..\n */\n-#define DIFF_OLD_NAME 0\n-#define DIFF_NEW_NAME 1\n+\n+/* #define DIFF_OLD_NAME 0 */\n+/* #define DIFF_NEW_NAME 1 */\n+\n+enum diff_name {\n+\tDIFF_OLD_NAME = 0,\n+\tDIFF_NEW_NAME\n+};\n \n static int gitdiff_verify_name(struct gitdiff_data *state,\n \t\t\t const char *line,\n-- \n2.39.1\n\n"},{"id":"471929","messageId":"20230210171338.81906-3-vinayakdev.sci@gmail.com","threadId":"59226","inReplyTo":"20230210171338.81906-1-vinayakdev.sci@gmail.com","subject":"[PATCH v2 2/3] {apply,alias}: convert pre-processor macros to enums","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-02-10T17:13:37Z","receivedAt":"2023-02-10T17:14:25Z","isPatch":true,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"Revert the changes made to alias.c in v1.\n\nSigned-off-by: Vinayak Dev \n\n---\n alias.c | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/alias.c b/alias.c\nindex 61ef2c0c54..deb871d59c 100644\n--- a/alias.c\n+++ b/alias.c\n@@ -44,14 +44,9 @@ void list_aliases(struct string_list *list)\n \tread_early_config(config_alias_cb, &data);\n }\n \n-/* #define SPLIT_CMDLINE_BAD_ENDING 1 */\n-/* #define SPLIT_CMDLINE_UNCLOSED_QUOTE 2 */\n-/* #define SPLIT_CMDLINE_ARGC_OVERFLOW 3 */\n-enum split_cmdline_error {\n-\tSPLIT_CMDLINE_BAD_ENDING = 1,\n-\tSPLIT_CMDLINE_UNCLOSED_QUOTE,\n-\tSPLIT_CMDLINE_ARGC_OVERFLOW\n-};\n+#define SPLIT_CMDLINE_BAD_ENDING 1\n+#define SPLIT_CMDLINE_UNCLOSED_QUOTE 2\n+#define SPLIT_CMDLINE_ARGC_OVERFLOW 3\n \n static const char *split_cmdline_errors[] = {\n \tN_(\"cmdline ends with \\\\\"),\n-- \n2.39.1\n\n"},{"id":"471930","messageId":"20230210171338.81906-4-vinayakdev.sci@gmail.com","threadId":"59226","inReplyTo":"20230210171338.81906-1-vinayakdev.sci@gmail.com","subject":"[PATCH v2 3/3] {apply,alias}: convert pre-processor macros to enums","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-02-10T17:13:38Z","receivedAt":"2023-02-10T17:14:36Z","isPatch":true,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"Change variables int side and int patch_method to enum in apply.c.\nThis allows for proper listing of enum constants during debugging.\n\nSigned-off-by: Vinayak Dev \n\n---\n apply.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 1e9cf2f4f2..8612bd44c8 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -205,9 +205,6 @@ struct fragment {\n * or deflated \"literal\".\n */\n #define binary_patch_method leading\n-/* #define BINARY_DELTA_DEFLATED 1 */\n-/* #define BINARY_LITERAL_DEFLATED 2 */\n-\n enum binary_type_deflated {\n \tBINARY_DELTA_DEFLATED = 1,\n \tBINARY_LITERAL_DEFLATED\n@@ -924,9 +921,6 @@ static int gitdiff_hdrend(struct gitdiff_data *state UNUSED,\n * to make sure..\n */\n \n-/* #define DIFF_OLD_NAME 0 */\n-/* #define DIFF_NEW_NAME 1 */\n-\n enum diff_name {\n \tDIFF_OLD_NAME = 0,\n \tDIFF_NEW_NAME\n@@ -936,7 +930,7 @@ static int gitdiff_verify_name(struct gitdiff_data *state,\n \t\t\t const char *line,\n \t\t\t int isnull,\n \t\t\t char **name,\n-\t\t\t int side)\n+\t\t\t enum diff_name side)\n {\n \tif (!*name && !isnull) {\n \t\t*name = find_name(state->root, line, NULL, state->p_value, TERM_TAB);\n@@ -1921,7 +1915,7 @@ static struct fragment *parse_binary_hunk(struct apply_state *state,\n \tint llen, used;\n \tunsigned long size = *sz_p;\n \tchar *buffer = *buf_p;\n-\tint patch_method;\n+\tenum binary_type_deflated patch_method;\n \tunsigned long origlen;\n \tchar *data = NULL;\n \tint hunk_size = 0;\n-- \n2.39.1\n\n"},{"id":"471950","messageId":"xmqq8rh5gox7.fsf@gitster.g","threadId":"59226","inReplyTo":"20230210171338.81906-1-vinayakdev.sci@gmail.com","subject":"Re: [PATCH v2 0/3] {apply,alias}: convert pre-processor macros to enums","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-10T22:01:56Z","receivedAt":"2023-02-10T22:02:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vinayak Dev writes:\n\n> Revert changes to alias.c, and change variable types in apply.c\n\nWhen you send out a rerolled series, you do not have to show your\npast mistakes. This [v2] is structured as a three patch series\nwhose first one makes a similar mess as what [v1] did, the second\none and the third one then revert some parts of that earlier mess.\n\nThat is not what you want to show your reviewers, and more\nimportantly, that is not what we want to record in our history.\nRerolling a series is your chance to pretend that you are much\nbetter programmer than you who wrote the [v1] patch. The review\nexchange is to help you do that. Please take advantage of that.\n\nYou may find \"git rebase -i\" is a useful tool to help you pretend\nthat you got to the ideal end result without these \"I tried this\nfirst, which was wrong in these points, which I correct in a\nsubsequent step\" steps.\n\nThanks.\n"},{"id":"471970","messageId":"CADE8NaqvP1JbLL89prOCu2Qo-ZVJTTN_CZjk7bJwzMctV9Z2Bw@mail.gmail.com","threadId":"59226","inReplyTo":"xmqq8rh5gox7.fsf@gitster.g","subject":"Re: [PATCH v2 0/3] {apply,alias}: convert pre-processor macros to enums","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-02-11T04:00:39Z","receivedAt":"2023-02-11T04:01:01Z","isPatch":true,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"On Sat, 11 Feb 2023 at 03:31, Junio C Hamano wrote:\n>\n> Vinayak Dev writes:\n>\n> > Revert changes to alias.c, and change variable types in apply.c\n>\n> When you send out a rerolled series, you do not have to show your\n> past mistakes. This [v2] is structured as a three patch series\n> whose first one makes a similar mess as what [v1] did, the second\n> one and the third one then revert some parts of that earlier mess.\n>\n> That is not what you want to show your reviewers, and more\n> importantly, that is not what we want to record in our history.\n> Rerolling a series is your chance to pretend that you are much\n> better programmer than you who wrote the [v1] patch. The review\n> exchange is to help you do that. Please take advantage of that.\n>\n> You may find \"git rebase -i\" is a useful tool to help you pretend\n> that you got to the ideal end result without these \"I tried this\n> first, which was wrong in these points, which I correct in a\n> subsequent step\" steps.\n>\n> Thanks.\n\nOK, I will keep that in mind before sending subsequent patches.\nShould I re-send [v2] after making corrections for this mistake?\nThat would make the corrections more obvious and the mistakes less.\n\nThanks a lot!\nVinayak\n"},{"id":"472100","messageId":"CAPig+cT98gscoD=Fh-QzJxruxdZmjGKW3gy95cA3xAokV7J4rw@mail.gmail.com","threadId":"59226","inReplyTo":"CADE8NaqvP1JbLL89prOCu2Qo-ZVJTTN_CZjk7bJwzMctV9Z2Bw@mail.gmail.com","subject":"Re: [PATCH v2 0/3] {apply,alias}: convert pre-processor macros to enums","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-02-15T00:04:28Z","receivedAt":"2023-02-15T00:04:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 10, 2023 at 11:00 PM Vinayak Dev wrote:\n> On Sat, 11 Feb 2023 at 03:31, Junio C Hamano wrote:\n> > Vinayak Dev writes:\n> > > Revert changes to alias.c, and change variable types in apply.c\n> >\n> > When you send out a rerolled series, you do not have to show your\n> > past mistakes. This [v2] is structured as a three patch series\n> > whose first one makes a similar mess as what [v1] did, the second\n> > one and the third one then revert some parts of that earlier mess.\n> >\n> > That is not what you want to show your reviewers, and more\n> > importantly, that is not what we want to record in our history.\n> > Rerolling a series is your chance to pretend that you are much\n> > better programmer than you who wrote the [v1] patch. The review\n> > exchange is to help you do that. Please take advantage of that.\n> >\n> > You may find \"git rebase -i\" is a useful tool to help you pretend\n> > that you got to the ideal end result without these \"I tried this\n> > first, which was wrong in these points, which I correct in a\n> > subsequent step\" steps.\n>\n> OK, I will keep that in mind before sending subsequent patches.\n> Should I re-send [v2] after making corrections for this mistake?\n> That would make the corrections more obvious and the mistakes less.\n\nYou should send a v3 which completely replaces v1 and v2.\n\nTo prepare v3, use the \"squash\" (or \"fixup\") command of `git rebase\n-i` to squash all three patches from v2 into a single patch, so that\nv3 consists of just one patch. The squashed patch should contain only\nchanges to \"apply.c\"; specifically, changing #define to `enum`, and\nchanging the variable declarations from `int` to `enum`.\n\nYou can also update the commit message of the squashed patch so that\nit explains the reason for the patch: specifically, the debugger will\ndisplay the values symbolically rather than as mere numbers.\n\nFinally, proofread the commit message and the patch itself, and resubmit as v3.\n"},{"id":"472127","messageId":"CADE8NaquOppesW5v9LMNE=TAu5J=bS9uCgkhkbu4QZ9JpoSLxg@mail.gmail.com","threadId":"59226","inReplyTo":"CAPig+cT98gscoD=Fh-QzJxruxdZmjGKW3gy95cA3xAokV7J4rw@mail.gmail.com","subject":"Re: [PATCH v2 0/3] {apply,alias}: convert pre-processor macros to enums","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-02-15T08:19:13Z","receivedAt":"2023-02-15T08:19:58Z","isPatch":true,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"On Wed, 15 Feb 2023 at 05:34, Eric Sunshine wrote:\n>\n> On Fri, Feb 10, 2023 at 11:00 PM Vinayak Dev wrote:\n\n> > Should I re-send [v2] after making corrections for this mistake?\n> > That would make the corrections more obvious and the mistakes less.\n>\n> You should send a v3 which completely replaces v1 and v2.\n>\n> To prepare v3, use the \"squash\" (or \"fixup\") command of `git rebase\n> -i` to squash all three patches from v2 into a single patch, so that\n> v3 consists of just one patch. The squashed patch should contain only\n> changes to \"apply.c\"; specifically, changing #define to `enum`, and\n> changing the variable declarations from `int` to `enum`.\n>\n> You can also update the commit message of the squashed patch so that\n> it explains the reason for the patch: specifically, the debugger will\n> display the values symbolically rather than as mere numbers.\n>\n> Finally, proofread the commit message and the patch itself, and resubmit as v3.\n\nOk! I have absolutely understood your points. I will roll out v3 with\nappropriate\nchanges, as you suggest.\n\nThanks a lot!\n Vinayak\n"}]}