postgrespro / pgsphere

PgSphere provides spherical data types, functions, operators, and indexing for PostgreSQL.
https://pgsphere.org
BSD 3-Clause "New" or "Revised" License
16 stars 15 forks source link

Add "pgindent" rule for make to run pgindent #71

Closed vitcpp closed 8 months ago

vitcpp commented 9 months ago

Implemented pgindent rule to run pgindent on the code. Utilities pgindent and pg_bsd_indent should be in the PATH or specified in the make command line: make PGINDENT=<...> PGBSDINDENT=<...> pgindent

vitcpp commented 8 months ago

@df7cb I sorted the typedefs and rebased the branch. Thank you!

esabol commented 8 months ago

Where do you get pgbsdindent? My postgresql-15.4/src/tools/pgindent/ directory only compiles the pgindent and pgperltidy executables....

Hmmm, the README in postgresql-15.4/src/tools/pgindent/ says to git clone https://git.postgresql.org/git/pg_bsd_indent.git. That compiles pg_bsd_indent, not pgbsdindent. I'm confused as to the difference. I presume they are compatible, but why is the name of the executable different?

Might be time to add a CONTRIBUTING.md file to the repo to explain this stuff?

df7cb commented 8 months ago

pg_bsd_indent got imported into the postgresql git, but only in the current master branch. The idea is that different branches might have different indentation rules or something.

https://git.postgresql.org/gitweb/?p=postgresql.git;a=tree;f=src/tools/pg_bsd_indent

(pg_bsd_indent is the correct spelling, pgbsdindent doesn't exist. But pgindent exists, for added confusion...)

df7cb commented 8 months ago

At the top of the pgindent/README file, there is a link to a blog post that explains how to use it on non-core code:

https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/tools/pgindent/README

http://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html

Dropping a link to at least the first one would make sense, perhaps as a comment on the Makefile pgindent rule.

esabol commented 8 months ago

Oops, I'm not sure why I thought the executable is named "pgbsdindent". The PR description and the changes to the Makefile clearly show its "pg_bsd_indent". I guess I was just temporarily blind. Anyway, thanks for clarifying, @df7cb.

I guess I'd still like to see some text added to the README.pg_sphere file or comment(s) added to the Makefile or a CONTRIBUTING.md file added to the repo which explains where/how to get the pgindent and pg_bsd_indent executables.

vitcpp commented 8 months ago

@esabol I think, we have to put some explanations to CONTRIBUTING.md but I propose to do it later. Now CONTRIBUTING.md is too old and not complete. It should be updated. What I propose to do right now is to put some information into the Makefile (above the rule). To run pgindent a postgresql source tree should be installed because it contains sources of pg_bsd_indent and pgindent, or these files should be somehow available in PATH by some other means. The pgindent, pg_bsd_indent binaries should be in the PATH.

I agree with @df7cb to put the link into the Makefile:
https://git.postgresql.org/gitweb/p=postgresql.git;a=blob;f=src/tools/pgindent/README

Now we do not use pgindent on a regular basis but it can be used when claiming some code formatting changes.

df7cb commented 8 months ago

Fwiw I plan to change the PG17 Debian packages to include pg_bsd_indent and pgindent, if that helps.

https://wiki.postgresql.org/wiki/Apt/FAQ#Development_snapshots

vitcpp commented 8 months ago

Rebased, fixed issues reported in the review