Re: [HACKERS] updated emacs configuration

2014-06-12 Thread Noah Misch
On Tue, Jun 10, 2014 at 10:36:07AM -0400, Tom Lane wrote:
 Noah Misch n...@leadboat.com writes:
  On Mon, Jun 09, 2014 at 09:04:02PM -0400, Peter Eisentraut wrote:
  I'd consider just getting rid of the
  
  (c-file-style . bsd)
  
  setting in .dir-locals.el and put the actual interesting settings from
  the style in there.
  
  Another alternative is to change emacs.samples to use
  hack-local-variables-hook instead, as described here:
  http://www.emacswiki.org/emacs/LocalVariables
 
  Those are reasonable alternatives.  The ultimate effect looks similar for 
  all
  three options, to the extent that I don't see a basis for forming a strong
  preference.  Do you have a recommendation?
 
 The more Ludd^H^H^Hcautious among us run with
   (setq enable-local-variables nil)
 and would rather not see the configuration recommendations overcomplicated
 due to an attempt to play nice with directory-local settings we're ignoring
 anyway.  So I'd vote for Peter's first option, ie, kluge up .dir-locals.el
 not the configuration code.

Seeing the two votes cast and only cosmetic differences between the options,
I'll just stick with my original -v1.  Thanks.

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-06-10 Thread Noah Misch
On Mon, Jun 09, 2014 at 09:04:02PM -0400, Peter Eisentraut wrote:
 On Sun, 2014-06-08 at 21:55 -0400, Noah Misch wrote:
  After upgrading to GNU Emacs 23.4.1 from a version predating directory-local
  variables, I saw switch/case indentation go on the fritz.  My hooks were
  issuing (c-set-style postgresql), but .dir-locals.el set it back to 
  plain
  bsd style.
 
 I'd consider just getting rid of the
 
 (c-file-style . bsd)
 
 setting in .dir-locals.el and put the actual interesting settings from
 the style in there.
 
 Another alternative is to change emacs.samples to use
 hack-local-variables-hook instead, as described here:
 http://www.emacswiki.org/emacs/LocalVariables

Those are reasonable alternatives.  The ultimate effect looks similar for all
three options, to the extent that I don't see a basis for forming a strong
preference.  Do you have a recommendation?

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-06-10 Thread Tom Lane
Noah Misch n...@leadboat.com writes:
 On Mon, Jun 09, 2014 at 09:04:02PM -0400, Peter Eisentraut wrote:
 On Sun, 2014-06-08 at 21:55 -0400, Noah Misch wrote:
 After upgrading to GNU Emacs 23.4.1 from a version predating directory-local
 variables, I saw switch/case indentation go on the fritz.  My hooks were
 issuing (c-set-style postgresql), but .dir-locals.el set it back to 
 plain
 bsd style.

 I'd consider just getting rid of the
 
 (c-file-style . bsd)
 
 setting in .dir-locals.el and put the actual interesting settings from
 the style in there.
 
 Another alternative is to change emacs.samples to use
 hack-local-variables-hook instead, as described here:
 http://www.emacswiki.org/emacs/LocalVariables

 Those are reasonable alternatives.  The ultimate effect looks similar for all
 three options, to the extent that I don't see a basis for forming a strong
 preference.  Do you have a recommendation?

The more Ludd^H^H^Hcautious among us run with
(setq enable-local-variables nil)
and would rather not see the configuration recommendations overcomplicated
due to an attempt to play nice with directory-local settings we're ignoring
anyway.  So I'd vote for Peter's first option, ie, kluge up .dir-locals.el
not the configuration code.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-06-09 Thread Tom Lane
Noah Misch n...@leadboat.com writes:
 On Wed, Aug 07, 2013 at 07:57:53AM -0400, Peter Eisentraut wrote:
 Did anyone have any outstanding concerns about this latest version?  I
 thought it looked ready to commit.

 After upgrading to GNU Emacs 23.4.1 from a version predating directory-local
 variables, I saw switch/case indentation go on the fritz.  My hooks were
 issuing (c-set-style postgresql), but .dir-locals.el set it back to plain
 bsd style.  The most-reasonable fix I found was to locally add c-file-style
 to ignored-local-variables.

It seems pretty ugly for this patch to be overriding our own
.dir-locals.el, especially in a way with as many potential side effects
as that.  Why can't we fix .dir-locals.el to play nice with this?
Is there a way for it to check for whether postgresql style is defined?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-06-09 Thread Noah Misch
On Mon, Jun 09, 2014 at 10:52:34AM -0400, Tom Lane wrote:
 Noah Misch n...@leadboat.com writes:
  On Wed, Aug 07, 2013 at 07:57:53AM -0400, Peter Eisentraut wrote:
  Did anyone have any outstanding concerns about this latest version?  I
  thought it looked ready to commit.
 
  After upgrading to GNU Emacs 23.4.1 from a version predating directory-local
  variables, I saw switch/case indentation go on the fritz.  My hooks were
  issuing (c-set-style postgresql), but .dir-locals.el set it back to 
  plain
  bsd style.  The most-reasonable fix I found was to locally add 
  c-file-style
  to ignored-local-variables.
 
 It seems pretty ugly for this patch to be overriding our own
 .dir-locals.el, especially in a way with as many potential side effects
 as that.  Why can't we fix .dir-locals.el to play nice with this?
 Is there a way for it to check for whether postgresql style is defined?

Nontrivial material in .dir-locals.el will tend to fire unsafe-local-variable
warnings.  Ugly situation, yes.  What are these many potential side effects
you have in mind?

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-06-09 Thread Peter Eisentraut
On Sun, 2014-06-08 at 21:55 -0400, Noah Misch wrote:
 After upgrading to GNU Emacs 23.4.1 from a version predating directory-local
 variables, I saw switch/case indentation go on the fritz.  My hooks were
 issuing (c-set-style postgresql), but .dir-locals.el set it back to plain
 bsd style.

I'd consider just getting rid of the

(c-file-style . bsd)

setting in .dir-locals.el and put the actual interesting settings from
the style in there.

Another alternative is to change emacs.samples to use
hack-local-variables-hook instead, as described here:
http://www.emacswiki.org/emacs/LocalVariables




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-06-08 Thread Noah Misch
On Wed, Aug 07, 2013 at 07:57:53AM -0400, Peter Eisentraut wrote:
 On 7/2/13 8:42 PM, Peter Eisentraut wrote:
  Updated files with changes:
  
  - adjusted fill-column to 78, per Noah
  - added c-file-style, per Andrew
  - support both postgresql and postgres directory names
  - use defun instead of lambda, per Dimitri
  - put Perl configuration back into emacs.samples, for Tom
  
  I also added configuration of c-auto-align-backslashes as well as label
  and statement-case-open to c-offsets-alist.  With those changes, the
  result of indent-region is now very very close to pgindent, with the
  main exception of the end-of-line de-indenting that pgindent does, which
  nobody likes anyway.
 
 Did anyone have any outstanding concerns about this latest version?  I
 thought it looked ready to commit.

After upgrading to GNU Emacs 23.4.1 from a version predating directory-local
variables, I saw switch/case indentation go on the fritz.  My hooks were
issuing (c-set-style postgresql), but .dir-locals.el set it back to plain
bsd style.  The most-reasonable fix I found was to locally add c-file-style
to ignored-local-variables.  c-file-style is the only setting appearing in
both emacs.samples and .dir-locals.el with non-identical values, so it alone
calls for this treatment.  Behavior looks sane in both Emacs 21.4.1 and Emacs
24.3.1, the version extrema I have at hand.  Emacs 24.3.1 is fine without this
due to recent c-before-hack-hook changes, but this does no harm.  Shall I add
this workaround to emacs.samples?

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/tools/editors/emacs.samples b/src/tools/editors/emacs.samples
index e469d55..a510afd 100644
--- a/src/tools/editors/emacs.samples
+++ b/src/tools/editors/emacs.samples
@@ -30,7 +30,12 @@
 (add-hook 'c-mode-hook
   (defun postgresql-c-mode-hook ()
 (when (string-match /postgres\\(ql\\)?/ buffer-file-name)
-  (c-set-style postgresql
+  (c-set-style postgresql)
+  ;; Don't override the style we just set with the style in
+  ;; `dir-locals-file'.  Emacs 23.4.1 needs this; it is obsolete,
+  ;; albeit harmless, by Emacs 24.3.1.
+  (set (make-local-variable 'ignored-local-variables)
+   (append '(c-file-style) ignored-local-variables)
 
 
 ;;; Perl files

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-02-06 Thread Peter Eisentraut
On 7/20/13, 9:06 PM, Greg Stark wrote:
 In theory this tool is promising though since it works by looking at
 the llvm bytecode to determine what the real syntax is. It should be
 able to handle the typedef issues we have with most of the the tools.

Since clang-format has no options to specify include file paths, I doubt
that it actually analyzes the file to the extent that it knows about
typedefs.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-02-06 Thread Andres Freund
On 2013-07-21 02:06:02 +0100, Greg Stark wrote:
 On Thu, Jun 27, 2013 at 10:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 
  AFAIR, no one has ever done a serious comparison to anything except GNU
  indent, and (at least at the time) it seemed to have bugs as bad as
  pgindent's, just different ones.  I'm certainly open to another choice
  as long as we don't lose on portability of the tool.  But who will do
  the legwork to test something else?
 
 Fwiw I played with clang-format a bit the other day. But I couldn't
 get it to resemble our coding style. It was strongly of the opinion
 that spacing within lines should be completely consistent and that
 meant it eliminated all spaces that lined up initializers and such.

FWIW, there's some work going on to add more options (like pg correct
differences between function definitions and declarations). So it might
develop into something usable for pg.

What I haven't seen in any tool yet is something to resemble our
indentation style for variable declarations. Do we feel that's a
dealbreaker for any tool?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-02-06 Thread Alvaro Herrera
Andres Freund wrote:

 What I haven't seen in any tool yet is something to resemble our
 indentation style for variable declarations. Do we feel that's a
 dealbreaker for any tool?

I find our style more aesthetically pleasing than any other I've seen;
but anyway the changes if we used a tool that formatted them differently
would cause way too much havoc to even be considered, IMO.

You know what I wouldn't mind losing?  The indentation of our
switch/case blocks.  It wastes too much valuable space at the left.
But then there are ~1000 uses of it in the tree so perhaps it'd be too
much trouble as well.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-02-06 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 What I haven't seen in any tool yet is something to resemble our
 indentation style for variable declarations. Do we feel that's a
 dealbreaker for any tool?

Well, we're unlikely to change pgindent's rules to conform to the
behavior of some random emacs mode, if that's what you mean ...

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-02-06 Thread Andres Freund
On 2014-02-06 10:04:54 -0500, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  What I haven't seen in any tool yet is something to resemble our
  indentation style for variable declarations. Do we feel that's a
  dealbreaker for any tool?
 
 Well, we're unlikely to change pgindent's rules to conform to the
 behavior of some random emacs mode, if that's what you mean ...

Nah, not for that ;). I mean I haven't seen any replacement for pgindent
that generates equivalent variable indentations.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-02-06 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 Nah, not for that ;). I mean I haven't seen any replacement for pgindent
 that generates equivalent variable indentations.

Seems odd.  I thought netbsd's indent was pretty much the granddaddy of
them all --- so you'd expect newer tools to be able to replicate that
style.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-02-06 Thread Andres Freund
On 2014-02-06 10:24:34 -0500, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  Nah, not for that ;). I mean I haven't seen any replacement for pgindent
  that generates equivalent variable indentations.
 
 Seems odd.  I thought netbsd's indent was pretty much the granddaddy of
 them all --- so you'd expect newer tools to be able to replicate that
 style.

I don't know, I guess not many projects use the indented variable names
in declarations style these days. But I guess it's not too hard to add.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-31 Thread Robert Haas
On Thu, Jan 30, 2014 at 11:06 PM, Bruce Momjian br...@momjian.us wrote:
 On Thu, Jan 30, 2014 at 03:36:48PM -0500, Bruce Momjian wrote:
 On Thu, Jan 30, 2014 at 03:32:27PM -0500, Robert Haas wrote:
  Or this:
 
  - mp_int_copy(a, b); /* ok: 0 = r  b */
  - mp_int_copy(q, a); /* ok: q = a   */
  + mp_int_copy(a, b); /* ok:  0 = r  b */
  + mp_int_copy(q, a); /* ok:  q = a */
 
  I do agree with you some of the changes-after-periods look like
  improvements; what I meant to say is that there is an awful lot of
  churn in this patch that is unrelated to that particular point.

 Let me work on a entab patch that does replacements in comments only
 after periods and post the results.  I should be done in an hour.

 OK, eight hours later, I have the results for only removing tabs after
 periods in comments:

 http://momjian.us/expire/entab_comment.v2.cdiff
 http://momjian.us/expire/entab_comment.v2.pdiff
 http://momjian.us/expire/entab_comment.v2.udiff

 The diff line counts are:

 64356 entab_comment.v2.cdiff
 17327 entab_comment.v2.pdiff
 35453 entab_comment.v2.udiff

 It represents 3903 lines changed.

That looks loads better.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-31 Thread Bruce Momjian
On Fri, Jan 31, 2014 at 08:58:21AM -0500, Robert Haas wrote:
  OK, eight hours later, I have the results for only removing tabs after
  periods in comments:
 
  http://momjian.us/expire/entab_comment.v2.cdiff
  http://momjian.us/expire/entab_comment.v2.pdiff
  http://momjian.us/expire/entab_comment.v2.udiff
 
  The diff line counts are:
 
  64356 entab_comment.v2.cdiff
  17327 entab_comment.v2.pdiff
  35453 entab_comment.v2.udiff
 
  It represents 3903 lines changed.
 
 That looks loads better.

OK.  I have updated entab.c to support this new ability as -m.  When
should it be run this against HEAD and supported back branches? Probably
when we run pgindent for 9.4.  I will need to modify pgindent too
because this new option will be run inside pgindent in the future.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-31 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 OK.  I have updated entab.c to support this new ability as -m.  When
 should it be run this against HEAD and supported back branches? Probably
 when we run pgindent for 9.4.

Yeah.  The whole point is to keep the branches in sync for patching,
so we need to do them all at the same time.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-31 Thread Bruce Momjian
On Fri, Jan 31, 2014 at 11:57:28AM -0500, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  OK.  I have updated entab.c to support this new ability as -m.  When
  should it be run this against HEAD and supported back branches? Probably
  when we run pgindent for 9.4.
 
 Yeah.  The whole point is to keep the branches in sync for patching,
 so we need to do them all at the same time.

Right.  Technically you could run entab -m on all the trees without
running pgindent, but I see no reason for double-churn and breaking
peoples' trees twice.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-30 Thread Robert Haas
On Wed, Jan 29, 2014 at 11:21 PM, Bruce Momjian br...@momjian.us wrote:
 On Wed, Jan 29, 2014 at 07:31:29PM -0500, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  I have cleaned up entab.c so I am ready to add a new option that removes
  tabs from only comments.  Would you like me to create that and provide a
  diff at a URL?  It would have to be run against all back branches.

 If you think you can actually tell the difference reliably in entab,
 sure, give it a go.

 OK, I have modified entab.c in a private patch to only process text
 inside comments, and not process leading whitespace, patch attached.  I
 basically ran 'entab -o -t4 -d' on the C files.

 The result are here, in context, plain, and unified format:

 http://momjian.us/expire/entab_comment.cdiff
 http://momjian.us/expire/entab_comment.pdiff
 http://momjian.us/expire/entab_comment.udiff

 and their line counts:

 89741 entab_comment.cdiff
 26351 entab_comment.pdiff
 50503 entab_comment.udiff

 I compute 6627 lines as modified.  What I did not do is handle _only_
 cases with periods before the tabs.  Should I try that?

I don't have any comment on that specific point, but I took a quick
look through one of these diffs and it looks like a lot of churn for
no improvement.  So I'm not sure what we want to do here, but I don't
think it's this.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-30 Thread Alvaro Herrera
Robert Haas escribió:
 On Wed, Jan 29, 2014 at 11:21 PM, Bruce Momjian br...@momjian.us wrote:

  I compute 6627 lines as modified.  What I did not do is handle _only_
  cases with periods before the tabs.  Should I try that?
 
 I don't have any comment on that specific point, but I took a quick
 look through one of these diffs and it looks like a lot of churn for
 no improvement.  So I'm not sure what we want to do here, but I don't
 think it's this.

I, on the contrary, think that the cases that are preceded by a period
are an improvement, and the rest are not (the opposite, actually, so
better not change those).  Maybe there are specific cases in which a
period-tab sequence should be kept, but that seems rarer.

I didn't check the entire diff obviously, so I can't comment on how good
about detecting comments the new entab code is.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-30 Thread Robert Haas
On Thu, Jan 30, 2014 at 3:04 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Robert Haas escribió:
 On Wed, Jan 29, 2014 at 11:21 PM, Bruce Momjian br...@momjian.us wrote:
  I compute 6627 lines as modified.  What I did not do is handle _only_
  cases with periods before the tabs.  Should I try that?

 I don't have any comment on that specific point, but I took a quick
 look through one of these diffs and it looks like a lot of churn for
 no improvement.  So I'm not sure what we want to do here, but I don't
 think it's this.

 I, on the contrary, think that the cases that are preceded by a period
 are an improvement, and the rest are not (the opposite, actually, so
 better not change those).  Maybe there are specific cases in which a
 period-tab sequence should be kept, but that seems rarer.

 I didn't check the entire diff obviously, so I can't comment on how good
 about detecting comments the new entab code is.

Well, the thing I really didn't like was this sort of thing:

- /* Oid subtype = PG_GETARG_OID(3); */
+ /* Oid  subtype = PG_GETARG_OID(3); */

Or this:

- * Original author: Simon Riggs  si...@2ndquadrant.com
- * Current maintainer: Simon Riggs
+ * Original author: Simon Riggs  si...@2ndquadrant.com
+ * Current maintainer:  Simon Riggs

Or this:

- * dirpath  - the directory name supplied on the command line
- * configpath  - optional configuration directory
- * envVarName  - the name of an environment variable to get if dirpath is NULL
+ * dirpath   - the directory name supplied on the command line
+ * configpath- optional configuration directory
+ * envVarName- the name of an environment variable to get if
dirpath is NULL

Or this:

- mp_int_copy(a, b); /* ok: 0 = r  b */
- mp_int_copy(q, a); /* ok: q = a   */
+ mp_int_copy(a, b); /* ok:  0 = r  b */
+ mp_int_copy(q, a); /* ok:  q = a */

I do agree with you some of the changes-after-periods look like
improvements; what I meant to say is that there is an awful lot of
churn in this patch that is unrelated to that particular point.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-30 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Robert Haas escribió:
 I don't have any comment on that specific point, but I took a quick
 look through one of these diffs and it looks like a lot of churn for
 no improvement.  So I'm not sure what we want to do here, but I don't
 think it's this.

 I, on the contrary, think that the cases that are preceded by a period
 are an improvement, and the rest are not (the opposite, actually, so
 better not change those).  Maybe there are specific cases in which a
 period-tab sequence should be kept, but that seems rarer.

I concur with Alvaro; the only cases I want to see changed are period
followed by a tab that would be exactly two spaces.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-30 Thread Bruce Momjian
On Thu, Jan 30, 2014 at 03:32:27PM -0500, Robert Haas wrote:
 Or this:
 
 - mp_int_copy(a, b); /* ok: 0 = r  b */
 - mp_int_copy(q, a); /* ok: q = a   */
 + mp_int_copy(a, b); /* ok:  0 = r  b */
 + mp_int_copy(q, a); /* ok:  q = a */
 
 I do agree with you some of the changes-after-periods look like
 improvements; what I meant to say is that there is an awful lot of
 churn in this patch that is unrelated to that particular point.

Let me work on a entab patch that does replacements in comments only
after periods and post the results.  I should be done in an hour.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-30 Thread Bruce Momjian
On Thu, Jan 30, 2014 at 03:36:48PM -0500, Bruce Momjian wrote:
 On Thu, Jan 30, 2014 at 03:32:27PM -0500, Robert Haas wrote:
  Or this:
  
  - mp_int_copy(a, b); /* ok: 0 = r  b */
  - mp_int_copy(q, a); /* ok: q = a   */
  + mp_int_copy(a, b); /* ok:  0 = r  b */
  + mp_int_copy(q, a); /* ok:  q = a */
  
  I do agree with you some of the changes-after-periods look like
  improvements; what I meant to say is that there is an awful lot of
  churn in this patch that is unrelated to that particular point.
 
 Let me work on a entab patch that does replacements in comments only
 after periods and post the results.  I should be done in an hour.

OK, eight hours later, I have the results for only removing tabs after
periods in comments:

http://momjian.us/expire/entab_comment.v2.cdiff
http://momjian.us/expire/entab_comment.v2.pdiff
http://momjian.us/expire/entab_comment.v2.udiff

The diff line counts are:

64356 entab_comment.v2.cdiff
17327 entab_comment.v2.pdiff
35453 entab_comment.v2.udiff

It represents 3903 lines changed.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-29 Thread Bruce Momjian
On Wed, Jan 29, 2014 at 12:53:02AM -0500, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  If this only affects a handful of places, then sure, let's go ahead
  and fix it.  But if it's going to create a massive enough diff that
  we've gotta think about back-patching it, then IMHO it's totally not
  worth it.
 
 A quick grep search says there are well over 3000 comment lines containing
 '.' followed by a tab.  grep isn't smart enough to tell if the tabs expand
 to exactly two spaces, but I bet the vast majority do.  So it might not
 be quite as bad as the 8.1 wrap-margin change, but it'd be extensive.

I have cleaned up entab.c so I am ready to add a new option that removes
tabs from only comments.  Would you like me to create that and provide a
diff at a URL?  It would have to be run against all back branches.

If it goes well, it could prove to be a way to incrementally improve
pgindent.  If not, I am afraid we are stuck with our current pgindent
output.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-29 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 I have cleaned up entab.c so I am ready to add a new option that removes
 tabs from only comments.  Would you like me to create that and provide a
 diff at a URL?  It would have to be run against all back branches.

If you think you can actually tell the difference reliably in entab,
sure, give it a go.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-29 Thread Bruce Momjian
On Wed, Jan 29, 2014 at 07:31:29PM -0500, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  I have cleaned up entab.c so I am ready to add a new option that removes
  tabs from only comments.  Would you like me to create that and provide a
  diff at a URL?  It would have to be run against all back branches.
 
 If you think you can actually tell the difference reliably in entab,
 sure, give it a go.

OK, I have modified entab.c in a private patch to only process text
inside comments, and not process leading whitespace, patch attached.  I
basically ran 'entab -o -t4 -d' on the C files.

The result are here, in context, plain, and unified format:

http://momjian.us/expire/entab_comment.cdiff
http://momjian.us/expire/entab_comment.pdiff
http://momjian.us/expire/entab_comment.udiff

and their line counts:

89741 entab_comment.cdiff
26351 entab_comment.pdiff
50503 entab_comment.udiff

I compute 6627 lines as modified.  What I did not do is handle _only_
cases with periods before the tabs.  Should I try that?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/src/tools/entab/entab.c b/src/tools/entab/entab.c
new file mode 100644
index 3b849f2..3fd2997
*** a/src/tools/entab/entab.c
--- b/src/tools/entab/entab.c
*** main(int argc, char **argv)
*** 51,63 
  {
  	int			tab_size = 8,
  min_spaces = 2,
  protect_quotes = FALSE,
  del_tabs = FALSE,
  clip_lines = FALSE,
  prv_spaces,
  col_in_tab,
  escaped,
! nxt_spaces;
  	char		in_line[BUFSIZ],
  out_line[BUFSIZ],
  			   *src,
--- 51,66 
  {
  	int			tab_size = 8,
  min_spaces = 2,
+ only_comments = FALSE,
  protect_quotes = FALSE,
  del_tabs = FALSE,
  clip_lines = FALSE,
+ in_comment = FALSE,
  prv_spaces,
  col_in_tab,
  escaped,
! nxt_spaces,
! leading_whitespace;
  	char		in_line[BUFSIZ],
  out_line[BUFSIZ],
  			   *src,
*** main(int argc, char **argv)
*** 74,80 
  	if (strcmp(cp, detab) == 0)
  		del_tabs = 1;
  
! 	while ((ch = getopt(argc, argv, cdhqs:t:)) != -1)
  		switch (ch)
  		{
  			case 'c':
--- 77,83 
  	if (strcmp(cp, detab) == 0)
  		del_tabs = 1;
  
! 	while ((ch = getopt(argc, argv, cdhoqs:t:)) != -1)
  		switch (ch)
  		{
  			case 'c':
*** main(int argc, char **argv)
*** 83,88 
--- 86,94 
  			case 'd':
  del_tabs = TRUE;
  break;
+ 			case 'o':
+ only_comments = TRUE;
+ break;
  			case 'q':
  protect_quotes = TRUE;
  break;
*** main(int argc, char **argv)
*** 97,102 
--- 103,109 
  fprintf(stderr, USAGE: %s [ -cdqst ] [file ...]\n\
  	-c (clip trailing whitespace)\n\
  	-d (delete tabs)\n\
+ 	-o (only comments)\n\
  	-q (protect quotes)\n\
  	-s minimum_spaces\n\
  	-t tab_width\n,
*** main(int argc, char **argv)
*** 134,146 
  			if (escaped == FALSE)
  quote_char = ' ';
  			escaped = FALSE;
  
  			/* process line */
  			while (*src != NUL)
  			{
  col_in_tab++;
  /* Is this a potential space/tab replacement? */
! if (quote_char == ' '  (*src == ' ' || *src == '\t'))
  {
  	if (*src == '\t')
  	{
--- 141,163 
  			if (escaped == FALSE)
  quote_char = ' ';
  			escaped = FALSE;
+ 			leading_whitespace = TRUE;
  
  			/* process line */
  			while (*src != NUL)
  			{
  col_in_tab++;
+ 
+ /* look backward so we handle slash-star-slash properly */
+ if (!in_comment  src  in_line 
+ 	*(src - 1) == '/'  *src == '*')
+ 	in_comment = TRUE;
+ else if (in_comment  *src == '*'  *(src + 1) == '/')
+ 	in_comment = FALSE;
+ 
  /* Is this a potential space/tab replacement? */
! if ((!only_comments || (in_comment  !leading_whitespace)) 
! 	quote_char == ' '  (*src == ' ' || *src == '\t'))
  {
  	if (*src == '\t')
  	{
*** main(int argc, char **argv)
*** 192,197 
--- 209,218 
  /* Not a potential space/tab replacement */
  else
  {
+ 	/* allow leading stars in comments */
+ 	if (leading_whitespace  *src != ' '  *src != '\t' 
+ 		(!in_comment || *src != '*'))
+ 		leading_whitespace = FALSE;
  	/* output accumulated spaces */
  	output_accumulated_spaces(prv_spaces, dst);
  	/* This can only happen in a quote. */

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Bruce Momjian
On Thu, Jun 27, 2013 at 05:31:54PM -0400, Tom Lane wrote:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
  Noah Misch wrote:
  Note that emacs and pgindent remain at odds over interior tabs in comments.
  When pgindent finds a double-space (typically after a sentence) ending at a
  tab stop, it replaces the double-space with a tab.  c-fill-paragraph will
  convert that tab to a *single* space, and that can be enough to change many
  line break positions.
 
  We should really stop pgindent from converting those double-spaces to
  tabs.  Those tabs are later changed to three or four spaces when wording
  of the comment is changed, and things start looking very odd.
 
 +1.  That's probably the single most annoying bit of behavior in pgindent.
 Being a two-spaces-after-a-period kind of guy, it might bite me more
 often than other people, but now that somebody else has brought it up...

Sorry I am just getting to this.  I actually have an easy fix for this,
and it is a feature of entab:

$ entab -h
USAGE: entab [ -cdqst ] [file ...]
-c (clip trailing whitespace)
-d (delete tabs)
-q (protect quotes)
-s minimum_spaces
-t tab_width

-s minimum_spaces defaults to 2, and pgindent doesn't change the
default. If we change the entab call in pgindent from

$entab -t4 -qc

to

$entab -t4 -qc -s3

we will no longer place a tab in this string, friend.  Hopefully
  --

even if there is a tab stop before the 'H'.  It will use a 3-space
break.  Does that help?  Other ideas?  How about requiring 4?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Bruce Momjian
On Thu, Jun 27, 2013 at 09:54:45PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  Should we be using entab -s 3?
 
 IIUC, that wouldn't affect leading whitespace at all.  What it would
 affect I think (outside of comment blocks) is whitespace between code
 and a same-line /* ... */ comment.  Personally I'd prefer that a
 tab-stop-aligned /* ... */ comment always have a tab before it, even
 if the expansion of the tab is only *one* space.  That is, in
 
 foo(bar,/* comment */
 bar1,   /* comment */
 bar12,  /* comment */
 bar123, /* comment */
 baz);   /* comment */
 
 I think each of those comments ought to have a tab before it, not
 space(s).  pgindent currently does this inconsistently --- the bar123
 line will have a space instead.  Moving to -s 3 would presumably make
 this worse (even less consistent) not better, since now the bar12 line
 would also be emitted with spaces not a tab.
 
 Inside a comment, though, probably the behavior of -s 3 would be just
 fine.  So the real problem here IMO is that use of tabs ought to be
 context sensitive (behavior inside a comment different from outside),
 and I don't think entab can do that.  I see though that it understands
 about C quoted strings, so maybe we could teach it about comments too?
 
 No idea whether astyle is any smarter about this.

I see I already asked about entab -s 3.  Let me see how hard it would be
to modify entab.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Alvaro Herrera
Bruce Momjian wrote:
 
 I see I already asked about entab -s 3.  Let me see how hard it would be
 to modify entab.

I cannot tell you what to waste your time on, of course, but I wonder if
it'd be better spent trying to make something like astyle work for us.
The entab/pg_bsd_indent combo is a bit long on the tooth by now.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Bruce Momjian
On Tue, Jan 28, 2014 at 06:39:43PM -0300, Alvaro Herrera wrote:
 Bruce Momjian wrote:
  
  I see I already asked about entab -s 3.  Let me see how hard it would be
  to modify entab.
 
 I cannot tell you what to waste your time on, of course, but I wonder if
 it'd be better spent trying to make something like astyle work for us.
 The entab/pg_bsd_indent combo is a bit long on the tooth by now.

I know I tried GNU indent long ago several times, and I thought astyle
didn't have the options we wanted, but it has been so long ago I can't
be sure.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 I see I already asked about entab -s 3.  Let me see how hard it would be
 to modify entab.

TBH I'm not sure we should be changing pgindent at this late date, even if
there is a good fix for its minor annoyances.  When you changed its wrap
behavior in 8.1, I spent the next several years cursing that decision
every time I had to back-patch something, because it caused apply failures
for just about every nontrivial patch.  Doing what we're talking about
here would be just as bad.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Bruce Momjian
On Tue, Jan 28, 2014 at 10:40:10PM -0500, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  I see I already asked about entab -s 3.  Let me see how hard it would be
  to modify entab.
 
 TBH I'm not sure we should be changing pgindent at this late date, even if
 there is a good fix for its minor annoyances.  When you changed its wrap
 behavior in 8.1, I spent the next several years cursing that decision
 every time I had to back-patch something, because it caused apply failures
 for just about every nontrivial patch.  Doing what we're talking about
 here would be just as bad.

Would you like to see how many comments get changed by the adjustment? 
Could we run it in all back branches, perhaps only on C comments?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 On Tue, Jan 28, 2014 at 10:40:10PM -0500, Tom Lane wrote:
 TBH I'm not sure we should be changing pgindent at this late date, even if
 there is a good fix for its minor annoyances.  When you changed its wrap
 behavior in 8.1, I spent the next several years cursing that decision
 every time I had to back-patch something, because it caused apply failures
 for just about every nontrivial patch.  Doing what we're talking about
 here would be just as bad.

 Would you like to see how many comments get changed by the adjustment? 
 Could we run it in all back branches, perhaps only on C comments?

Hm.  Reindenting the active back branches would fix the back-patching
issue, but it would also be a pain in the rear for anybody carrying
out-of-tree patches; which is probably the majority of our packagers.

On the third hand, updating such patches would only be a one-time
chore (as long as we fix pgindent only *once*, not anytime the mood
strikes us), so maybe it wouldn't be impossible.  Especially if we
constrain ourselves to just fixing tabs vs. spaces, so that patch
--ignore-whitespace could be used to apply old patches.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Robert Haas
On Tue, Jan 28, 2014 at 11:03 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Bruce Momjian br...@momjian.us writes:
 On Tue, Jan 28, 2014 at 10:40:10PM -0500, Tom Lane wrote:
 TBH I'm not sure we should be changing pgindent at this late date, even if
 there is a good fix for its minor annoyances.  When you changed its wrap
 behavior in 8.1, I spent the next several years cursing that decision
 every time I had to back-patch something, because it caused apply failures
 for just about every nontrivial patch.  Doing what we're talking about
 here would be just as bad.

 Would you like to see how many comments get changed by the adjustment?
 Could we run it in all back branches, perhaps only on C comments?

 Hm.  Reindenting the active back branches would fix the back-patching
 issue, but it would also be a pain in the rear for anybody carrying
 out-of-tree patches; which is probably the majority of our packagers.

 On the third hand, updating such patches would only be a one-time
 chore (as long as we fix pgindent only *once*, not anytime the mood
 strikes us), so maybe it wouldn't be impossible.  Especially if we
 constrain ourselves to just fixing tabs vs. spaces, so that patch
 --ignore-whitespace could be used to apply old patches.

If we do it once, we're likely to do it again; and even if we only do
it once, so what?  It's still a nuisance.

If this only affects a handful of places, then sure, let's go ahead
and fix it.  But if it's going to create a massive enough diff that
we've gotta think about back-patching it, then IMHO it's totally not
worth it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Andres Freund
On 2014-01-28 18:39:43 -0300, Alvaro Herrera wrote:
 Bruce Momjian wrote:
  
  I see I already asked about entab -s 3.  Let me see how hard it would be
  to modify entab.
 
 I cannot tell you what to waste your time on, of course, but I wonder if
 it'd be better spent trying to make something like astyle work for us.
 The entab/pg_bsd_indent combo is a bit long on the tooth by now.

I've wondered about getting the required options into clang's
clang-format. That would guarantee it's parsing things the right way and
they have many of the required options already and are adding others
(http://llvm-reviews.chandlerc.com/D2610).

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2014-01-28 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 If this only affects a handful of places, then sure, let's go ahead
 and fix it.  But if it's going to create a massive enough diff that
 we've gotta think about back-patching it, then IMHO it's totally not
 worth it.

A quick grep search says there are well over 3000 comment lines containing
'.' followed by a tab.  grep isn't smart enough to tell if the tabs expand
to exactly two spaces, but I bet the vast majority do.  So it might not
be quite as bad as the 8.1 wrap-margin change, but it'd be extensive.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-08-07 Thread Peter Eisentraut
On 7/2/13 8:42 PM, Peter Eisentraut wrote:
 Updated files with changes:
 
 - adjusted fill-column to 78, per Noah
 - added c-file-style, per Andrew
 - support both postgresql and postgres directory names
 - use defun instead of lambda, per Dimitri
 - put Perl configuration back into emacs.samples, for Tom
 
 I also added configuration of c-auto-align-backslashes as well as label
 and statement-case-open to c-offsets-alist.  With those changes, the
 result of indent-region is now very very close to pgindent, with the
 main exception of the end-of-line de-indenting that pgindent does, which
 nobody likes anyway.

Did anyone have any outstanding concerns about this latest version?  I
thought it looked ready to commit.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-08-07 Thread Dimitri Fontaine
Peter Eisentraut pete...@gmx.net writes:
 Did anyone have any outstanding concerns about this latest version?  I
 thought it looked ready to commit.

I wanted to have another round at trying it here and get a better
understanding at my failures of the previous time, but that's not a
blocker at all. I had another look at it and it looks ready to me too.

Minor detail: you seem to have forgotten to apply the postgres(ql)?
regexp to the GNU Make mode in the emacs.samples file.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-07-20 Thread Greg Stark
On Thu, Jun 27, 2013 at 10:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 AFAIR, no one has ever done a serious comparison to anything except GNU
 indent, and (at least at the time) it seemed to have bugs as bad as
 pgindent's, just different ones.  I'm certainly open to another choice
 as long as we don't lose on portability of the tool.  But who will do
 the legwork to test something else?

Fwiw I played with clang-format a bit the other day. But I couldn't
get it to resemble our coding style. It was strongly of the opinion
that spacing within lines should be completely consistent and that
meant it eliminated all spaces that lined up initializers and such.

In theory this tool is promising though since it works by looking at
the llvm bytecode to determine what the real syntax is. It should be
able to handle the typedef issues we have with most of the the tools.


-- 
greg


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-07-02 Thread Peter Eisentraut
Updated files with changes:

- adjusted fill-column to 78, per Noah
- added c-file-style, per Andrew
- support both postgresql and postgres directory names
- use defun instead of lambda, per Dimitri
- put Perl configuration back into emacs.samples, for Tom

I also added configuration of c-auto-align-backslashes as well as label
and statement-case-open to c-offsets-alist.  With those changes, the
result of indent-region is now very very close to pgindent, with the
main exception of the end-of-line de-indenting that pgindent does, which
nobody likes anyway.

;; see also src/tools/editors/emacs.samples for more complete settings

((c-mode . ((c-basic-offset . 4)
(c-file-style . bsd)
(fill-column . 78)
(indent-tabs-mode . t)
(tab-width . 4)))
 (dsssl-mode . ((indent-tabs-mode . nil)))
 (nxml-mode . ((indent-tabs-mode . nil)))
 (perl-mode . ((perl-indent-level . 4)
   (perl-continued-statement-offset . 4)
   (perl-continued-brace-offset . 4)
   (perl-brace-offset . 0)
   (perl-brace-imaginary-offset . 0)
   (perl-label-offset . -2)
   (tab-width . 4)))
 (sgml-mode . ((fill-column . 78)
   (indent-tabs-mode . nil
;; -*- mode: emacs-lisp -*-

;; This file contains code to set up Emacs to edit PostgreSQL source
;; code.  Copy these snippets into your .emacs file or equivalent, or
;; use load-file to load this file directly.
;;
;; Note also that there is a .dir-locals.el file at the top of the
;; PostgreSQL source tree, which contains many of the settings shown
;; here (but not all, mainly because not all settings are allowed as
;; local variables).  So for light editing, you might not need any
;; additional Emacs configuration.


;;; C files

;; Style that matches the formatting used by
;; src/tools/pgindent/pgindent.  Many extension projects also use this
;; style.
(c-add-style postgresql
 '(bsd
   (c-auto-align-backslashes . nil)
   (c-basic-offset . 4)
   (c-offsets-alist . ((case-label . +)
   (label . -)
   (statement-case-open . +)))
   (fill-column . 78)
   (indent-tabs-mode . t)
   (tab-width . 4)))

(add-hook 'c-mode-hook
  (defun postgresql-c-mode-hook ()
(when (string-match /postgres\\(ql\\)?/ buffer-file-name)
  (c-set-style postgresql


;;; Perl files

;; Style that matches the formatting used by
;; src/tools/pgindent/perltidyrc.
(defun pgsql-perl-style ()
  Perl style adjusted for PostgreSQL project
  (interactive)
  (setq perl-brace-imaginary-offset 0)
  (setq perl-brace-offset 0)
  (setq perl-continued-brace-offset 4)
  (setq perl-continued-statement-offset 4)
  (setq perl-indent-level 4)
  (setq perl-label-offset -2)
  (setq tab-width 4))

(add-hook 'perl-mode-hook
  (defun postgresql-perl-mode-hook ()
 (when (string-match /postgres\\(ql\\)?/ buffer-file-name)
   (pgsql-perl-style


;;; documentation files

(add-hook 'sgml-mode-hook
  (defun postgresql-sgml-mode-hook ()
 (when (string-match /postgres\\(ql\\)?/ buffer-file-name)
   (setq fill-column 78)
   (setq indent-tabs-mode nil)
   (setq sgml-basic-offset 1


;;; Makefiles

;; use GNU make mode instead of plain make mode
(add-to-list 'auto-mode-alist '(/postgresql/.*Makefile.* . 
makefile-gmake-mode))
(add-to-list 'auto-mode-alist '(/postgresql/.*\\.mk\\' . makefile-gmake-mode))

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Alvaro Herrera
Noah Misch wrote:

 Note that emacs and pgindent remain at odds over interior tabs in comments.
 When pgindent finds a double-space (typically after a sentence) ending at a
 tab stop, it replaces the double-space with a tab.  c-fill-paragraph will
 convert that tab to a *single* space, and that can be enough to change many
 line break positions.

We should really stop pgindent from converting those double-spaces to
tabs.  Those tabs are later changed to three or four spaces when wording
of the comment is changed, and things start looking very odd.

Really, we should get out of patched BSD indent entirely already.  How
about astyle, for instance?  I'm told that with some sensible options,
the diff to what we have now is not very large, and several things
actually become sensible (such as function pointer decls, which are
messed up rather badly by pgindent)

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Bruce Momjian
On Thu, Jun 27, 2013 at 03:51:15PM -0400, Alvaro Herrera wrote:
 Noah Misch wrote:
 
  Note that emacs and pgindent remain at odds over interior tabs in comments.
  When pgindent finds a double-space (typically after a sentence) ending at a
  tab stop, it replaces the double-space with a tab.  c-fill-paragraph will
  convert that tab to a *single* space, and that can be enough to change many
  line break positions.
 
 We should really stop pgindent from converting those double-spaces to
 tabs.  Those tabs are later changed to three or four spaces when wording
 of the comment is changed, and things start looking very odd.
 
 Really, we should get out of patched BSD indent entirely already.  How
 about astyle, for instance?  I'm told that with some sensible options,
 the diff to what we have now is not very large, and several things
 actually become sensible (such as function pointer decls, which are
 messed up rather badly by pgindent)

Sounds good to me.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Noah Misch wrote:
 Note that emacs and pgindent remain at odds over interior tabs in comments.
 When pgindent finds a double-space (typically after a sentence) ending at a
 tab stop, it replaces the double-space with a tab.  c-fill-paragraph will
 convert that tab to a *single* space, and that can be enough to change many
 line break positions.

 We should really stop pgindent from converting those double-spaces to
 tabs.  Those tabs are later changed to three or four spaces when wording
 of the comment is changed, and things start looking very odd.

+1.  That's probably the single most annoying bit of behavior in pgindent.
Being a two-spaces-after-a-period kind of guy, it might bite me more
often than other people, but now that somebody else has brought it up...

 Really, we should get out of patched BSD indent entirely already.  How
 about astyle, for instance?  I'm told that with some sensible options,
 the diff to what we have now is not very large, and several things
 actually become sensible (such as function pointer decls, which are
 messed up rather badly by pgindent)

AFAIR, no one has ever done a serious comparison to anything except GNU
indent, and (at least at the time) it seemed to have bugs as bad as
pgindent's, just different ones.  I'm certainly open to another choice
as long as we don't lose on portability of the tool.  But who will do
the legwork to test something else?

Probably the principal argument against switching to a different tool
has been that whitespace changes would complicate back-patching, but
I don't see why we couldn't solve that by re-indenting all the live back
branches at the time of the switch.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Andres Freund
On 2013-06-27 17:31:54 -0400, Tom Lane wrote:
  Really, we should get out of patched BSD indent entirely already.  How
  about astyle, for instance?  I'm told that with some sensible options,
  the diff to what we have now is not very large, and several things
  actually become sensible (such as function pointer decls, which are
  messed up rather badly by pgindent)
 
 AFAIR, no one has ever done a serious comparison to anything except GNU
 indent, and (at least at the time) it seemed to have bugs as bad as
 pgindent's, just different ones.  I'm certainly open to another choice
 as long as we don't lose on portability of the tool.  But who will do
 the legwork to test something else?

I think before doing any serious testing we would need to lay out how
many changes and what changes in formatting we would allow and what kind
of enforced formatting rules we think are required.

Being at least one of the persons having mentioned astyle to Alvaro, I
had tested that once and I thought the results were resembling something
reasonable after an hour of fiddling or so. But there were certain
things that I could not be make it do during that. The only thing I
remember now was reducing the indentation of parameters to the left if
the line length got to long. Now, I personally think that's an
anti-feature, but I am not sure if others think differently.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 I think before doing any serious testing we would need to lay out how
 many changes and what changes in formatting we would allow and what kind
 of enforced formatting rules we think are required.

Well, we certainly never applied any such analysis to pgindent.  I
personally don't mind leaving corner cases to the judgment of the tool
author ... of course, what's a corner case and what's important may be
in the eye of the beholder.  But it's surely a bug, for instance, that
pgindent is so clueless about function pointers.

 Being at least one of the persons having mentioned astyle to Alvaro, I
 had tested that once and I thought the results were resembling something
 reasonable after an hour of fiddling or so. But there were certain
 things that I could not be make it do during that. The only thing I
 remember now was reducing the indentation of parameters to the left if
 the line length got to long. Now, I personally think that's an
 anti-feature, but I am not sure if others think differently.

I never particularly cared for that behavior either.  It probably made
sense back in the video-terminal days, when your view of a program was
80 columns period.  These days I think most people can use a wider
window at need --- not that I want to adopt wider lines as standard, but
the readability tradeoff between not having lines wrap versus messing up
the indentation seems like it's probably different now.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Alvaro Herrera
Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
 
  Being at least one of the persons having mentioned astyle to Alvaro, I
  had tested that once and I thought the results were resembling something
  reasonable after an hour of fiddling or so. But there were certain
  things that I could not be make it do during that. The only thing I
  remember now was reducing the indentation of parameters to the left if
  the line length got to long. Now, I personally think that's an
  anti-feature, but I am not sure if others think differently.
 
 I never particularly cared for that behavior either.  It probably made
 sense back in the video-terminal days, when your view of a program was
 80 columns period.

I've never liked that either; I am a fan of keeping things to 80
columns, but when things get longer I prefer my editor to wrap them to
the next line without the silly de-indent (or not wrap, if I tell it not
to.)

Another benefit of more modern tools is that there's no need for a
typedef file, which is great when you're trying to indent after a patch
which adds some more typedefs that are not listed in the file.

 These days I think most people can use a wider
 window at need --- not that I want to adopt wider lines as standard, but
 the readability tradeoff between not having lines wrap versus messing up
 the indentation seems like it's probably different now.

Agreed.  I would be sad if we adopted a policy of sloppiness on width.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Bruce Momjian
On Thu, Jun 27, 2013 at 05:31:54PM -0400, Tom Lane wrote:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
  Noah Misch wrote:
  Note that emacs and pgindent remain at odds over interior tabs in comments.
  When pgindent finds a double-space (typically after a sentence) ending at a
  tab stop, it replaces the double-space with a tab.  c-fill-paragraph will
  convert that tab to a *single* space, and that can be enough to change many
  line break positions.
 
  We should really stop pgindent from converting those double-spaces to
  tabs.  Those tabs are later changed to three or four spaces when wording
  of the comment is changed, and things start looking very odd.
 
 +1.  That's probably the single most annoying bit of behavior in pgindent.
 Being a two-spaces-after-a-period kind of guy, it might bite me more
 often than other people, but now that somebody else has brought it up...

That might be caused by entab, actually.  I think there are two cases
here:

*  six-space indented line
*  two spaces in text that happen to land on a tab stop

entab doesn't distinguish them, but it certainly could.  In fact, it
already has an option for that:

-s  Minimum spaces needed to replace with a tab (default = 2).

Should we be using entab -s 3?  That would make a six-space indent be a
tab and two spaces, but it would certainly handle the period-space-space
case.  I actually don't remember people complaining about this.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Should we be using entab -s 3?

IIUC, that wouldn't affect leading whitespace at all.  What it would
affect I think (outside of comment blocks) is whitespace between code
and a same-line /* ... */ comment.  Personally I'd prefer that a
tab-stop-aligned /* ... */ comment always have a tab before it, even
if the expansion of the tab is only *one* space.  That is, in

foo(bar,/* comment */
bar1,   /* comment */
bar12,  /* comment */
bar123, /* comment */
baz);   /* comment */

I think each of those comments ought to have a tab before it, not
space(s).  pgindent currently does this inconsistently --- the bar123
line will have a space instead.  Moving to -s 3 would presumably make
this worse (even less consistent) not better, since now the bar12 line
would also be emitted with spaces not a tab.

Inside a comment, though, probably the behavior of -s 3 would be just
fine.  So the real problem here IMO is that use of tabs ought to be
context sensitive (behavior inside a comment different from outside),
and I don't think entab can do that.  I see though that it understands
about C quoted strings, so maybe we could teach it about comments too?

No idea whether astyle is any smarter about this.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-27 Thread Bruce Momjian
On Thu, Jun 27, 2013 at 09:54:45PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  Should we be using entab -s 3?
 
 IIUC, that wouldn't affect leading whitespace at all.  What it would
 affect I think (outside of comment blocks) is whitespace between code
 and a same-line /* ... */ comment.  Personally I'd prefer that a
 tab-stop-aligned /* ... */ comment always have a tab before it, even
 if the expansion of the tab is only *one* space.  That is, in
 
 foo(bar,/* comment */
 bar1,   /* comment */
 bar12,  /* comment */
 bar123, /* comment */
 baz);   /* comment */
 
 I think each of those comments ought to have a tab before it, not
 space(s).  pgindent currently does this inconsistently --- the bar123
 line will have a space instead.  Moving to -s 3 would presumably make
 this worse (even less consistent) not better, since now the bar12 line
 would also be emitted with spaces not a tab.

I think you have grasped the issue.  :-)

 Inside a comment, though, probably the behavior of -s 3 would be just
 fine.  So the real problem here IMO is that use of tabs ought to be
 context sensitive (behavior inside a comment different from outside),
 and I don't think entab can do that.  I see though that it understands
 about C quoted strings, so maybe we could teach it about comments too?

Yes, that could be done.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-26 Thread Dimitri Fontaine
Peter Eisentraut pete...@gmx.net writes:
 emacs -q src/backend/commands/extension.c
 emacs -q -l ../emacs.samples src/backend/commands/extension.c

 Well, the first one uses 8-space tabs, the second 4-space tabs, so they
 look completely different.  I'm not sure what you are trying to point
 out.

With the .dir-locals.el in place, the first one should be using 4-space
tabs too, right?

   - Name the lambda used in the hook for easier removing / reference

 Interesting, I had never thought of that.  I don't see that used in
 Emacs source code or core packages, however.  Do you have a reference
 that this is recommended practice?

Well a friend of mine pointed that out to me one day, and he's an Emacs
and Gnus commiter. The thing is that you're not guaranteed that the same
lambda source code will evaluate to the same object each time, and that
would prevent you to remove-hook.

 $ git clone git://git.postgresql.org/git/postgresql.git
 Cloning into 'postgresql'...

I can reproduce that here. I don't know why I have those postgres dirs
then, and I'm pretty confused about my round of testing now.

-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-26 Thread Jan Urbański

On 26/06/13 10:51, Dimitri Fontaine wrote:

Peter Eisentraut pete...@gmx.net writes:

$ git clone git://git.postgresql.org/git/postgresql.git
Cloning into 'postgresql'...


I can reproduce that here. I don't know why I have those postgres dirs
then, and I'm pretty confused about my round of testing now.


Maybe you cloned from GitHub, where the mirrored repository is called 
'postgres'?


Cheers,
Jan



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-26 Thread Noah Misch
On Tue, Jun 25, 2013 at 11:17:47PM -0400, Peter Eisentraut wrote:
 On Sun, 2013-06-23 at 16:03 -0400, Noah Misch wrote:
   ((c-mode . ((c-basic-offset . 4)
   (fill-column . 79)
  
  I don't know whether you'd consider it to fall within the scope of this
  update, but 78 is the fill-column setting that matches pgindent.
 
 Well, well, well.  I did some extensive tests some time ago when that
 setting was added.  I have a suspicion that this could be related to the
 recent pgindent changes (which everyone claims were no changes).  I'm
 going to have to research that some more.

pgindent has not changed the following xlog.c comment since April 2011, but
fill-column 77 or 79 changes it:

/*
 * fullPageWrites is the master copy used by all backends to determine
 * whether to write full-page to WAL, instead of using process-local 
one.
 * This is required because, when full_page_writes is changed by SIGHUP,
 * we must WAL-log it before it actually affects WAL-logging by 
backends.
 * Checkpointer sets at startup or after SIGHUP.
 */

Note that emacs and pgindent remain at odds over interior tabs in comments.
When pgindent finds a double-space (typically after a sentence) ending at a
tab stop, it replaces the double-space with a tab.  c-fill-paragraph will
convert that tab to a *single* space, and that can be enough to change many
line break positions.

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-25 Thread Peter Eisentraut
On Fri, 2013-06-21 at 11:20 +0200, Dimitri Fontaine wrote:
 I've tested that on a new git clone and with the `emacs -q` command so
 as not to load any of my local setup. While the indentation seemed ok,
 the placement of the comments seems way off:
 
 Compare what you see using those commands:
 
 emacs -q src/backend/commands/extension.c
 emacs -q -l ../emacs.samples src/backend/commands/extension.c

Well, the first one uses 8-space tabs, the second 4-space tabs, so they
look completely different.  I'm not sure what you are trying to point
out.

 A couple of notes about your emacs.sample file:
 
   - Name the lambda used in the hook for easier removing / reference

Interesting, I had never thought of that.  I don't see that used in
Emacs source code or core packages, however.  Do you have a reference
that this is recommended practice?

   - A fresh git clone will create a directory named postgres, so I did
 change your /postgresql/ regex to /postgres/ in my attached version

Huh?

$ git clone git://git.postgresql.org/git/postgresql.git
Cloning into 'postgresql'...




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-25 Thread Peter Eisentraut
On Sun, 2013-06-23 at 16:11 -0400, Andrew Dunstan wrote:
  The idea is a very good one in principle, but my short experiment with 
  the provided .dir-locals.el didn't give me BSD style brace 
  indentation. It works if we can do those unsafe things, but then we 
  surely don't want to have a user prompted to accept the settings each 
  time. If .dir-locals.el won't do what we need on its own, it seems to 
  me hardly worth having.

The main win will be to get 4-space tabs for casual observers, so the
files don't look crappy to newbies.

 However, it does work if you add this at the beginning of the c-mode list:
 
  (c-file-style . bsd)

You are right, that should be added.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-25 Thread Peter Eisentraut
On Sun, 2013-06-23 at 16:03 -0400, Noah Misch wrote:
  ((c-mode . ((c-basic-offset . 4)
  (fill-column . 79)
 
 I don't know whether you'd consider it to fall within the scope of this
 update, but 78 is the fill-column setting that matches pgindent.

Well, well, well.  I did some extensive tests some time ago when that
setting was added.  I have a suspicion that this could be related to the
recent pgindent changes (which everyone claims were no changes).  I'm
going to have to research that some more.

   (sgml-mode . ((fill-column . 79)
 (indent-tabs-mode . nil
 
 SGML fill-column practice has varied widely.  I estimate 72 is more the norm,
 though I personally favor longer lines like we use in source code comments.

I have no strong feelings about this.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-25 Thread Peter Eisentraut
On Sun, 2013-06-23 at 16:37 -0400, Tom Lane wrote:
 I'm un-thrilled with this as well, though for a slightly different
 reason: we have a policy that the PG sources should be tool agnostic,
 and in fact removed file-local emacs settings awhile back because of
 that.

We don't want to keep the tool settings in the sources, but what's wrong
with keeping them next to the sources?  We have
src/tools/editor/emacs.samples already.  This just moves some of the
contents to .dir-locals.el, where it would be more useful.

 Even though I use emacs, I would much rather keep such settings
 in my personal library.  (TBH, I keep enable-local-variables turned
 off, and would thus get no benefit from the proposed file anyhow.)

No one is required to use this, and seasoned hackers aren't exactly the
target audience anyway.

 Another thing I'm not too happy about is that the proposed patch
 summarily discards the information we had about how to work with older
 emacs versions.  I'm not sure it will actually break anybody's setup,
 but that would depend on whether .dir-locals.el was added before or
 after the last round of rewriting of c-mode.

I think you are confusing two separate changes.  The proposed addition
of .dir-locals.el is entirely optional.  (Well, I have removed the Perl
settings from emacs.samples.  We can keep them if disabling local
variables is a wide-spread practice.)  The C mode settings in the new
emacs.samples file are just as functional as the old ones, except they
are arguably better.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-23 Thread Andrew Dunstan


On 06/13/2013 09:27 PM, Peter Eisentraut wrote:

I think the suggested emacs configuration snippets in
src/tools/editors/emacs.samples no longer represent current best
practices.  I have come up with some newer things that I'd like to
propose for review.

First, I propose adding a .dir-locals.el file to the top-level directory
with basic emacs settings.  These get applied automatically.  This
especially covers the particular tab and indentation settings that
PostgreSQL uses.  With this, casual developers will not need to modify
any of their emacs settings.

(In the attachment, .dir-locals.el is called _dir-locals.el so that it
doesn't get lost.  To clarify, it goes into the same directory that
contains configure.in.)

With that, emacs.samples can be shrunk significantly.  The only real
reason to keep is that that c-offsets-alist and (more dubiously)
sgml-basic-offset cannot be set from .dir-locals.el because they are not
safe.  I have also removed many of the redundant examples and settled
on a hook-based solution.

I think together this setup would be significantly simpler and more
practical.



The idea is a very good one in principle, but my short experiment with 
the provided .dir-locals.el didn't give me BSD style brace indentation. 
It works if we can do those unsafe things, but then we surely don't 
want to have a user prompted to accept the settings each time. If 
.dir-locals.el won't do what we need on its own, it seems to me hardly 
worth having.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-23 Thread Noah Misch
On Thu, Jun 13, 2013 at 09:27:07PM -0400, Peter Eisentraut wrote:
 I think the suggested emacs configuration snippets in
 src/tools/editors/emacs.samples no longer represent current best
 practices.  I have come up with some newer things that I'd like to
 propose for review.

 ((c-mode . ((c-basic-offset . 4)
 (fill-column . 79)

I don't know whether you'd consider it to fall within the scope of this
update, but 78 is the fill-column setting that matches pgindent.

  (sgml-mode . ((fill-column . 79)
(indent-tabs-mode . nil

SGML fill-column practice has varied widely.  I estimate 72 is more the norm,
though I personally favor longer lines like we use in source code comments.

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-23 Thread Andrew Dunstan


On 06/23/2013 03:43 PM, Andrew Dunstan wrote:




The idea is a very good one in principle, but my short experiment with 
the provided .dir-locals.el didn't give me BSD style brace 
indentation. It works if we can do those unsafe things, but then we 
surely don't want to have a user prompted to accept the settings each 
time. If .dir-locals.el won't do what we need on its own, it seems to 
me hardly worth having.



However, it does work if you add this at the beginning of the c-mode list:

(c-file-style . bsd)


cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-23 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 The idea is a very good one in principle, but my short experiment with 
 the provided .dir-locals.el didn't give me BSD style brace indentation. 
 It works if we can do those unsafe things, but then we surely don't 
 want to have a user prompted to accept the settings each time. If 
 .dir-locals.el won't do what we need on its own, it seems to me hardly 
 worth having.

I'm un-thrilled with this as well, though for a slightly different
reason: we have a policy that the PG sources should be tool agnostic,
and in fact removed file-local emacs settings awhile back because of
that.  Even though I use emacs, I would much rather keep such settings
in my personal library.  (TBH, I keep enable-local-variables turned off,
and would thus get no benefit from the proposed file anyhow.)

Another thing I'm not too happy about is that the proposed patch
summarily discards the information we had about how to work with older
emacs versions.  I'm not sure it will actually break anybody's setup,
but that would depend on whether .dir-locals.el was added before or
after the last round of rewriting of c-mode.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-21 Thread Dimitri Fontaine
Hi,

Peter Eisentraut pete...@gmx.net writes:
 I think the suggested emacs configuration snippets in
 src/tools/editors/emacs.samples no longer represent current best
 practices.  I have come up with some newer things that I'd like to
 propose for review.

Thanks for doing that!

 First, I propose adding a .dir-locals.el file to the top-level directory
 with basic emacs settings.  These get applied automatically.  This
 especially covers the particular tab and indentation settings that
 PostgreSQL uses.  With this, casual developers will not need to modify
 any of their emacs settings.

I've tested that on a new git clone and with the `emacs -q` command so
as not to load any of my local setup. While the indentation seemed ok,
the placement of the comments seems way off:

Compare what you see using those commands:

emacs -q src/backend/commands/extension.c
emacs -q -l ../emacs.samples src/backend/commands/extension.c

(When using macosx, you might have to replace the 'emacs' binary
 location with /Applications/Emacs.app/Contents/MacOS/Emacs).

I did also test on doc/src/sgml/extend.sgml and some Makefile, only with
using the emacs.samples file content though.

 With that, emacs.samples can be shrunk significantly.  The only real
 reason to keep is that that c-offsets-alist and (more dubiously)
 sgml-basic-offset cannot be set from .dir-locals.el because they are not
 safe.  I have also removed many of the redundant examples and settled
 on a hook-based solution.

A couple of notes about your emacs.sample file:

  - Name the lambda used in the hook for easier removing / reference

  - A fresh git clone will create a directory named postgres, so I did
change your /postgresql/ regex to /postgres/ in my attached version

 I think together this setup would be significantly simpler and more
 practical.

Agreed.
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

;; -*- mode: emacs-lisp -*-

;; This file contains code to set up Emacs to edit PostgreSQL source
;; code.  Copy these snippets into your .emacs file or equivalent, or
;; use load-file to load this file directly.
;;
;; Note also that there is a .dir-locals.el file at the top of the
;; PostgreSQL source tree, which contains many of the settings shown
;; here.  So for light editing, you might not need any additional
;; Emacs configuration.


;;; C files

;; Style that matches the formatting used by
;; src/tools/pgindent/pgindent.  Many extension projects also use this
;; style.
(c-add-style postgresql
 '(bsd
   (c-basic-offset . 4)
   (c-offsets-alist . ((case-label . +)))
   (fill-column . 79)
   (indent-tabs-mode . t)
   (tab-width . 4)))

(add-hook 'c-mode-hook
  (defun postgresql-c-mode-hook ()
(when (string-match /postgres/ buffer-file-name)
  (c-set-style postgresql


;;; documentation files

(add-hook 'sgml-mode-hook
   (defun postgresql-sgml-mode-hook ()
 (when (string-match /postgres/ buffer-file-name)
   (setq fill-column 79)
   (setq indent-tabs-mode nil)
   (setq sgml-basic-offset 1


;;; Makefiles

;; use GNU make mode instead of plain make mode
(add-to-list 'auto-mode-alist '(/postgres/.*Makefile.* . makefile-gmake-mode))
(add-to-list 'auto-mode-alist '(/postgres/.*\\.mk\\' . makefile-gmake-mode))

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] updated emacs configuration

2013-06-14 Thread Daniel Farina
On Thu, Jun 13, 2013 at 6:27 PM, Peter Eisentraut pete...@gmx.net wrote:
 First, I propose adding a .dir-locals.el file to the top-level directory
 with basic emacs settings.  These get applied automatically.  This
 especially covers the particular tab and indentation settings that
 PostgreSQL uses.  With this, casual developers will not need to modify
 any of their emacs settings.

Yes please.  I've had the pgsql stuff in my .emacs for-ever (ever since I
was a student and compelled to do homework on Postgres) and knew the
magical rules about naming the directory, but it always felt so dirty
and very much a 'you need to know the trick' level of intimacy.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers