'converts internal representation to "..."' comment is confusing
Hello hackers,
I found "..." confusing in some comments, so this patch changes it to
"cstring". Which seems to be the intention after all.
Best regards,
Steve
Attachments:
0001-Fix-.-in-comments.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-.-in-comments.patchDownload
From cb1792c45ea9a2fbd2c08e185653b60dc262a17d Mon Sep 17 00:00:00 2001
From: steve-chavez <stevechavezast@gmail.com>
Date: Sun, 14 May 2023 18:00:49 -0300
Subject: [PATCH] Fix "..." in comments
"..." is confusing, change it to cstring.
---
src/backend/utils/adt/name.c | 4 ++--
src/backend/utils/adt/tags | 1 +
src/backend/utils/adt/varlena.c | 8 ++++----
3 files changed, 7 insertions(+), 6 deletions(-)
create mode 120000 src/backend/utils/adt/tags
diff --git a/src/backend/utils/adt/name.c b/src/backend/utils/adt/name.c
index 79c1b90030..c136eabdbc 100644
--- a/src/backend/utils/adt/name.c
+++ b/src/backend/utils/adt/name.c
@@ -38,7 +38,7 @@
/*
- * namein - converts "..." to internal representation
+ * namein - converts cstring to internal representation
*
* Note:
* [Old] Currently if strlen(s) < NAMEDATALEN, the extra chars are nulls
@@ -65,7 +65,7 @@ namein(PG_FUNCTION_ARGS)
}
/*
- * nameout - converts internal representation to "..."
+ * nameout - converts internal representation to cstring
*/
Datum
nameout(PG_FUNCTION_ARGS)
diff --git a/src/backend/utils/adt/tags b/src/backend/utils/adt/tags
new file mode 120000
index 0000000000..8d6288c7b2
--- /dev/null
+++ b/src/backend/utils/adt/tags
@@ -0,0 +1 @@
+./../../../../tags
\ No newline at end of file
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index b571876468..b895413c1d 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -571,7 +571,7 @@ bytea_string_agg_finalfn(PG_FUNCTION_ARGS)
}
/*
- * textin - converts "..." to internal representation
+ * textin - converts cstring to internal representation
*/
Datum
textin(PG_FUNCTION_ARGS)
@@ -582,7 +582,7 @@ textin(PG_FUNCTION_ARGS)
}
/*
- * textout - converts internal representation to "..."
+ * textout - converts internal representation to cstring
*/
Datum
textout(PG_FUNCTION_ARGS)
@@ -626,7 +626,7 @@ textsend(PG_FUNCTION_ARGS)
/*
- * unknownin - converts "..." to internal representation
+ * unknownin - converts cstring to internal representation
*/
Datum
unknownin(PG_FUNCTION_ARGS)
@@ -638,7 +638,7 @@ unknownin(PG_FUNCTION_ARGS)
}
/*
- * unknownout - converts internal representation to "..."
+ * unknownout - converts internal representation to cstring
*/
Datum
unknownout(PG_FUNCTION_ARGS)
--
2.34.1
Steve Chavez <steve@supabase.io> writes:
I found "..." confusing in some comments, so this patch changes it to
"cstring". Which seems to be the intention after all.
Those comments are Berkeley-era, making them probably a decade older
than the "cstring" pseudotype (invented in b663f3443). Perhaps what
you suggest is an improvement, but I'm not sure that appealing to
original intent can make the case.
regards, tom lane
Thanks a lot for the clarification!
The "..." looks enigmatic right now. I think cstring would save newcomers
some head-scratching.
Open to suggestions though.
Best regards,
Steve
On Sun, 14 May 2023 at 22:36, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Show quoted text
Steve Chavez <steve@supabase.io> writes:
I found "..." confusing in some comments, so this patch changes it to
"cstring". Which seems to be the intention after all.Those comments are Berkeley-era, making them probably a decade older
than the "cstring" pseudotype (invented in b663f3443). Perhaps what
you suggest is an improvement, but I'm not sure that appealing to
original intent can make the case.regards, tom lane
On Sun, May 14, 2023 at 9:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Steve Chavez <steve@supabase.io> writes:
I found "..." confusing in some comments, so this patch changes it to
"cstring". Which seems to be the intention after all.Those comments are Berkeley-era, making them probably a decade older
than the "cstring" pseudotype (invented in b663f3443). Perhaps what
you suggest is an improvement, but I'm not sure that appealing to
original intent can make the case.
FWIW, it does seem like an improvement to me.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hello hackers,
Tom, could we apply this patch since Robert agrees it's an improvement?
Best regards,
Steve
On Tue, 16 May 2023 at 07:49, Robert Haas <robertmhaas@gmail.com> wrote:
Show quoted text
On Sun, May 14, 2023 at 9:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Steve Chavez <steve@supabase.io> writes:
I found "..." confusing in some comments, so this patch changes it to
"cstring". Which seems to be the intention after all.Those comments are Berkeley-era, making them probably a decade older
than the "cstring" pseudotype (invented in b663f3443). Perhaps what
you suggest is an improvement, but I'm not sure that appealing to
original intent can make the case.FWIW, it does seem like an improvement to me.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 24/06/2023 23:52, Steve Chavez wrote:
On Tue, 16 May 2023 at 07:49, Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>> wrote:On Sun, May 14, 2023 at 9:37 PM Tom Lane <tgl@sss.pgh.pa.us
<mailto:tgl@sss.pgh.pa.us>> wrote:Steve Chavez <steve@supabase.io <mailto:steve@supabase.io>> writes:
I found "..." confusing in some comments, so this patch changes
it to
"cstring". Which seems to be the intention after all.
Those comments are Berkeley-era, making them probably a decade older
than the "cstring" pseudotype (invented in b663f3443). Perhaps what
you suggest is an improvement, but I'm not sure that appealing to
original intent can make the case.FWIW, it does seem like an improvement to me.
Tom, could we apply this patch since Robert agrees it's an improvement?
Looking around at other input/output functions, we're not very
consistent, there are many variants of "converts string to [datatype]",
"converts C string to [datatype]", and "input routine for [datatype]".
They are all fine, even though they're inconsistent. Doesn't seem worth
the code churn to change them.
Anyway, I agree this patch is an improvement, so applied. Thanks!
--
Heikki Linnakangas
Neon (https://neon.tech)