git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] abspath.h is created.
@ 2022-09-27 16:08 skrab-sah via GitGitGadget
  2022-09-28  0:40 ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: skrab-sah via GitGitGadget @ 2022-09-27 16:08 UTC (permalink / raw)
  To: git; +Cc: skrab-sah, skrab-sah

From: skrab-sah <skrab.sah@gmail.com>

replaced declaration of abspath.c from cache.h to abspath.h.
abspath.h is  generated by using makeheaders tool.

Signed-off-by: skrab-sah <skrab.sah@gmail.com>
---
    abspath.h file is generated by makeheaders tool
    
     1. we don't need to commit the file.
     2. added routin for abspath.c in Makefile.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1345%2Fskrab-sah%2Fmaster-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1345/skrab-sah/master-v1
Pull-Request: https://github.com/git/git/pull/1345

 abspath.c | 10 ++++++++++
 abspath.h |  9 +++++++++
 cache.h   | 21 +--------------------
 3 files changed, 20 insertions(+), 20 deletions(-)
 create mode 100644 abspath.h

diff --git a/abspath.c b/abspath.c
index 39e06b58486..1c163bbe651 100644
--- a/abspath.c
+++ b/abspath.c
@@ -262,6 +262,16 @@ char *absolute_pathdup(const char *path)
 	return strbuf_detach(&sb, NULL);
 }
 
+/*
+ * Concatenate "prefix" (if len is non-zero) and "path", with no
+ * connecting characters (so "prefix" should end with a "/").
+ * Unlike prefix_path, this should be used if the named file does
+ * not have to interact with index entry; i.e. name of a random file
+ * on the filesystem.
+ *
+ * The return value is always a newly allocated string (even if the
+ * prefix was empty).
+ */
 char *prefix_filename(const char *pfx, const char *arg)
 {
 	struct strbuf path = STRBUF_INIT;
diff --git a/abspath.h b/abspath.h
new file mode 100644
index 00000000000..edebc3a53ba
--- /dev/null
+++ b/abspath.h
@@ -0,0 +1,9 @@
+/* \aThis file was automatically generated.  Do not edit! */
+#undef INTERFACE
+char *prefix_filename(const char *pfx,const char *arg);
+char *absolute_pathdup(const char *path);
+const char *absolute_path(const char *path);
+char *real_pathdup(const char *path,int die_on_error);
+char *strbuf_realpath_forgiving(struct strbuf *resolved,const char *path,int die_on_error);
+char *strbuf_realpath(struct strbuf *resolved,const char *path,int die_on_error);
+int is_directory(const char *path);
diff --git a/cache.h b/cache.h
index 26ed03bd6de..e226dbcc7d5 100644
--- a/cache.h
+++ b/cache.h
@@ -646,18 +646,6 @@ const char *setup_git_directory(void);
 char *prefix_path(const char *prefix, int len, const char *path);
 char *prefix_path_gently(const char *prefix, int len, int *remaining, const char *path);
 
-/*
- * Concatenate "prefix" (if len is non-zero) and "path", with no
- * connecting characters (so "prefix" should end with a "/").
- * Unlike prefix_path, this should be used if the named file does
- * not have to interact with index entry; i.e. name of a random file
- * on the filesystem.
- *
- * The return value is always a newly allocated string (even if the
- * prefix was empty).
- */
-char *prefix_filename(const char *prefix, const char *path);
-
 int check_filename(const char *prefix, const char *name);
 void verify_filename(const char *prefix,
 		     const char *name,
@@ -1299,14 +1287,7 @@ static inline int is_absolute_path(const char *path)
 {
 	return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
 }
-int is_directory(const char *);
-char *strbuf_realpath(struct strbuf *resolved, const char *path,
-		      int die_on_error);
-char *strbuf_realpath_forgiving(struct strbuf *resolved, const char *path,
-				int die_on_error);
-char *real_pathdup(const char *path, int die_on_error);
-const char *absolute_path(const char *path);
-char *absolute_pathdup(const char *path);
+#include "abspath.h"
 const char *remove_leading_path(const char *in, const char *prefix);
 const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
 int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);

base-commit: 4fd6c5e44459e6444c2cd93383660134c95aabd1
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] abspath.h is created.
  2022-09-27 16:08 [PATCH] abspath.h is created skrab-sah via GitGitGadget
@ 2022-09-28  0:40 ` Junio C Hamano
  2022-09-28  7:32   ` Skrab Sah
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2022-09-28  0:40 UTC (permalink / raw)
  To: skrab-sah via GitGitGadget; +Cc: git, skrab-sah

"skrab-sah via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/abspath.h b/abspath.h
> new file mode 100644
> index 00000000000..edebc3a53ba
> --- /dev/null
> +++ b/abspath.h
> @@ -0,0 +1,9 @@
> +/* \aThis file was automatically generated.  Do not edit! */

No thanks.

I suspect this change is taking us in a wrong direction.  People
grep for function and struct declarations in <*.h> files and expect
to find the associated comments.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] abspath.h is created.
  2022-09-28  0:40 ` Junio C Hamano
@ 2022-09-28  7:32   ` Skrab Sah
  2022-09-28 15:39     ` Taylor Blau
  2022-09-28 17:58     ` Junio C Hamano
  0 siblings, 2 replies; 5+ messages in thread
From: Skrab Sah @ 2022-09-28  7:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: skrab-sah via GitGitGadget, git

What is wrong here and what should i do to make it correct.

On Wed, 28 Sept 2022 at 06:11, Junio C Hamano <gitster@pobox.com> wrote:
>
> "skrab-sah via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > diff --git a/abspath.h b/abspath.h
> > new file mode 100644
> > index 00000000000..edebc3a53ba
> > --- /dev/null
> > +++ b/abspath.h
> > @@ -0,0 +1,9 @@
> > +/*  This file was automatically generated.  Do not edit! */
>
> No thanks.
>
> I suspect this change is taking us in a wrong direction.  People
> grep for function and struct declarations in <*.h> files and expect
> to find the associated comments.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] abspath.h is created.
  2022-09-28  7:32   ` Skrab Sah
@ 2022-09-28 15:39     ` Taylor Blau
  2022-09-28 17:58     ` Junio C Hamano
  1 sibling, 0 replies; 5+ messages in thread
From: Taylor Blau @ 2022-09-28 15:39 UTC (permalink / raw)
  To: Skrab Sah; +Cc: Junio C Hamano, skrab-sah via GitGitGadget, git

Hi Skrab,

On Wed, Sep 28, 2022 at 01:02:22PM +0530, Skrab Sah wrote:
> What is wrong here and what should i do to make it correct.

I suspect that Junio's concern is that it appears the tool you're using
to auto-generate these .h files moves the comments away from the
declaration and into the header.

I imagine that such a change would be improved if:

  - You kept the comment attached to the function declaration (and
    ensured that it was present in the header file).

  - And we dropped the "this file is auto-generated by ..." comment,
    since I don't imagine that folks will be interested in adding a new
    tool to the development toolchain.

Thanks.


Taylor

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] abspath.h is created.
  2022-09-28  7:32   ` Skrab Sah
  2022-09-28 15:39     ` Taylor Blau
@ 2022-09-28 17:58     ` Junio C Hamano
  1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2022-09-28 17:58 UTC (permalink / raw)
  To: Skrab Sah; +Cc: skrab-sah via GitGitGadget, git

Skrab Sah <skrab.sah@gmail.com> writes:

> What is wrong here and what should i do to make it correct.

Almost everything in what the patch is wrong.

 * It creates <abspath.h> as a tracked file, but has a funny comment
   with control-G in it that says "do not edit"

 * It is entirely unclear how the contents of <abspath.h> is meant
   to evolve, if the contributors are forbidden to edit it, and
   without any instruction how to update it.

 * The new <abspath.h> lost all function comments, grouping by
   category, etc. that were in the original <cache.h>.

 * There is an "#undef INTERFACE" that has no use to us, those who
   develop Git, for no clearly explained reason.

 * The proposed log message says what it did, but it does not even
   try to justify why it is a good idea to do so.

 * There is no need to create a new <abspath.h> in the first place.
   If we were in an alternate world where we did not have
   <abspath.c>, it may be reasonable to add <abspath.h> along with
   it, instead of adding declarations for new functions and types in
   <cache.h>.  But the difference falls into "once it is in, it is
   not worth the patch churn to change it" category.

The easiest way to make it correct is to drop it, I guess.

Thanks.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-28 18:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-27 16:08 [PATCH] abspath.h is created skrab-sah via GitGitGadget
2022-09-28  0:40 ` Junio C Hamano
2022-09-28  7:32   ` Skrab Sah
2022-09-28 15:39     ` Taylor Blau
2022-09-28 17:58     ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).