Re: [PATCH] convert any hard coded .gitmodules file string to the MACRO

3 messages, 2 authors, 2017-08-02 · open the first message on its own page

Re: [PATCH] convert any hard coded .gitmodules file string to the MACRO

From: Junio C Hamano <hidden>
Date: 2017-08-01 20:26:42

Stefan Beller [off-list ref] writes:
quoted
quoted
  @@ -233,18 +233,18 @@ void gitmodules_config(void)
                strbuf_addstr(&gitmodules_path, "/.gitmodules");

Did you mean to also change "/.gitmodules" ??
Goog point. We should pick that up as well. However as we do not have
a macro for that, we'd have to have 2 calls to strbuf API

    strbuf_addch(&sb, '/');
    strbuf_addstr(&sb, GITMODULES);
Ehh, doesn't string literal concatenation work here?  I.e. something
like:

    strbuf_addstr(&gitmodules_path, "/" GITMODULES_FILE);

quoted
quoted
                if (pos < 0) { /* .gitmodules not found or isn't merged */
                        pos = -1 - pos;
                        if (active_nr > pos) {  /* there is a .gitmodules
*/

It might also be nice to change the literals in the comments to
use the macro.
The reason you want this patch is not like we want to make it easy
to rename the file to ".gitprojects" later, right?  The patch is
about avoiding misspelled string constant, like "/.gitmdoules",
without getting caught by the compiler, no?

Assuming that I am correctly guessing the intention, I think it is a
bad idea to rename these in the comments.

Re: [PATCH] convert any hard coded .gitmodules file string to the MACRO

From: Brandon Williams <hidden>
Date: 2017-08-02 17:26:42

On 08/01, Junio C Hamano wrote:
Stefan Beller [off-list ref] writes:
quoted
quoted
quoted
  @@ -233,18 +233,18 @@ void gitmodules_config(void)
                strbuf_addstr(&gitmodules_path, "/.gitmodules");

Did you mean to also change "/.gitmodules" ??
Goog point. We should pick that up as well. However as we do not have
a macro for that, we'd have to have 2 calls to strbuf API

    strbuf_addch(&sb, '/');
    strbuf_addstr(&sb, GITMODULES);
Ehh, doesn't string literal concatenation work here?  I.e. something
like:

    strbuf_addstr(&gitmodules_path, "/" GITMODULES_FILE);

quoted
quoted
quoted
                if (pos < 0) { /* .gitmodules not found or isn't merged */
                        pos = -1 - pos;
                        if (active_nr > pos) {  /* there is a .gitmodules
*/

It might also be nice to change the literals in the comments to
use the macro.
The reason you want this patch is not like we want to make it easy
to rename the file to ".gitprojects" later, right?  The patch is
about avoiding misspelled string constant, like "/.gitmdoules",
without getting caught by the compiler, no?
Yes, it was mostly about preventing mistakes and having the compiler
help you out a bit, so changing the comments isn't really needed.
Assuming that I am correctly guessing the intention, I think it is a
bad idea to rename these in the comments.
-- 
Brandon Williams

Re: [PATCH] convert any hard coded .gitmodules file string to the MACRO

From: Brandon Williams <hidden>
Date: 2017-08-02 17:46:24

On 08/01, Junio C Hamano wrote:
Stefan Beller [off-list ref] writes:
quoted
quoted
quoted
  @@ -233,18 +233,18 @@ void gitmodules_config(void)
                strbuf_addstr(&gitmodules_path, "/.gitmodules");

Did you mean to also change "/.gitmodules" ??
Goog point. We should pick that up as well. However as we do not have
a macro for that, we'd have to have 2 calls to strbuf API

    strbuf_addch(&sb, '/');
    strbuf_addstr(&sb, GITMODULES);
Ehh, doesn't string literal concatenation work here?  I.e. something
like:

    strbuf_addstr(&gitmodules_path, "/" GITMODULES_FILE);
Also this doesn't really matter much since this line is removed latter
on in the series, but I'll go with the string literal concatenation for
the intermediate state.
quoted
quoted
quoted
                if (pos < 0) { /* .gitmodules not found or isn't merged */
                        pos = -1 - pos;
                        if (active_nr > pos) {  /* there is a .gitmodules
*/

It might also be nice to change the literals in the comments to
use the macro.
The reason you want this patch is not like we want to make it easy
to rename the file to ".gitprojects" later, right?  The patch is
about avoiding misspelled string constant, like "/.gitmdoules",
without getting caught by the compiler, no?

Assuming that I am correctly guessing the intention, I think it is a
bad idea to rename these in the comments.
-- 
Brandon Williams
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help