[PATCH] Convert formatprg to a global-local option.
Sung Pae
A few months ago a question was posed to the vim_use group about the use
of the `formatprg` option [1]. Specifically, is it possible to pass the
value of `textwidth` to the filter program?
Generally, one wants to use different filter parameters when working
on different file types, so the simplest solution would be to set
`formatprg` during the FileType event. Unfortunately, `formatprg` is a
global-only option, so there cannot be different values for different
file types and buffers.
This patch converts `formatprg` into a global-local option, allowing for
this functionality. Most other filterprg options (`equalprg`, `grepprg`,
`keywordprg`, and `makeprg`) are already global-local, so there is
precedent for this change.
More details in the patch header, following inline and as an attachment.
Cheers,
Sung Pae
[1]: http://groups.google.com/group/vim_use/browse_thread/thread/ffca45a9c8dd4da1
--
From 3a37fa9fbe743d75b4bf7b76635ebd26e1cd0210 Mon Sep 17 00:00:00 2001
From: guns <se...@sungpae.com>
Date: Sun, 7 Aug 2011 04:49:45 -0500
Subject: [PATCH] Convert formatprg to a global-local option.
Since the purpose of the formatprg option is to allow external programs
to reflow non-source text, it is useful to have different formatprgs set
for different documents.
For instance, you may want to use `par` globally:
set formatprg=par\ 80
For mail messages, you would want to wrap to 72 characters and add quote
handling options:
autocmd FileType mail
\ setlocal textwidth=72 formatprg=par\ q1\ 72
And if you were working on a complex plain-text document, with a
left-hand gutter, justification, and non-standard tab expansions:
autocmd BufRead,BufNewFile *.thesis
\ setlocal tw=78 fp=par\ h3jT3\ 78
Note that while all of this could be done with a formatexpr, that option
is intended for formatting source code and is generally more complicated
to configure.
---
runtime/doc/options.txt | 2 +-
src/buffer.c | 1 +
src/normal.c | 6 +++---
src/option.c | 19 ++++++++++++++++++-
src/option.h | 1 +
src/proto/option.pro | 1 +
src/structs.h | 1 +
7 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/runtime/doc/options.txt b/runtime/doc/options.txt
index 3bfc05a..8bddb84 100644
--- a/runtime/doc/options.txt
+++ b/runtime/doc/options.txt
@@ -3210,7 +3210,7 @@ A jump table for the options with a short description can be found at |Q_op|.
*'formatprg'* *'fp'*
'formatprg' 'fp' string (default "")
- global
+ global or local to buffer |global-local|
{not in Vi}
The name of an external program that will be used to format the lines
selected with the |gq| operator. The program must take the input on
diff --git a/src/buffer.c b/src/buffer.c
index 24cbc06..ae9db49 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1828,6 +1828,7 @@ free_buf_options(buf, free_p_ff)
clear_string_option(&buf->b_p_efm);
#endif
clear_string_option(&buf->b_p_ep);
+ clear_string_option(&buf->b_p_fp);
clear_string_option(&buf->b_p_path);
clear_string_option(&buf->b_p_tags);
#ifdef FEAT_INS_EXPAND
diff --git a/src/normal.c b/src/normal.c
index c028fea..1740a2f 100644
--- a/src/normal.c
+++ b/src/normal.c
@@ -2060,7 +2060,7 @@ do_pending_operator(cap, old_col, gui_yank)
op_formatexpr(oap); /* use expression */
else
#endif
- if (*p_fp != NUL)
+ if (*get_formatprg() != NUL)
op_colon(oap); /* use external command */
else
op_format(oap, FALSE); /* use internal function */
@@ -2222,10 +2222,10 @@ op_colon(oap)
}
else if (oap->op_type == OP_FORMAT)
{
- if (*p_fp == NUL)
+ if (*get_formatprg() == NUL)
stuffReadbuff((char_u *)"fmt");
else
- stuffReadbuff(p_fp);
+ stuffReadbuff(get_formatprg());
stuffReadbuff((char_u *)"\n']");
}
diff --git a/src/option.c b/src/option.c
index 4db3027..a4c2ede 100644
--- a/src/option.c
+++ b/src/option.c
@@ -111,6 +111,7 @@
#define PV_FF OPT_BUF(BV_FF)
#define PV_FLP OPT_BUF(BV_FLP)
#define PV_FO OPT_BUF(BV_FO)
+#define PV_FP OPT_BOTH(OPT_BUF(BV_FP))
#ifdef FEAT_AUTOCMD
# define PV_FT OPT_BUF(BV_FT)
#endif
@@ -1208,7 +1209,7 @@ static struct vimoption
{(char_u *)"^\\s*\\d\\+[\\]:.)}\\t ]\\s*",
(char_u *)0L} SCRIPTID_INIT},
{"formatprg", "fp", P_STRING|P_EXPAND|P_VI_DEF|P_SECURE,
- (char_u *)&p_fp, PV_NONE,
+ (char_u *)&p_fp, PV_FP,
{(char_u *)"", (char_u *)0L} SCRIPTID_INIT},
{"fsync", "fs", P_BOOL|P_SECURE|P_VI_DEF,
#ifdef HAVE_FSYNC
@@ -5296,6 +5297,7 @@ check_buf_options(buf)
check_string_option(&buf->b_p_efm);
#endif
check_string_option(&buf->b_p_ep);
+ check_string_option(&buf->b_p_fp);
check_string_option(&buf->b_p_path);
check_string_option(&buf->b_p_tags);
#ifdef FEAT_INS_EXPAND
@@ -9438,6 +9440,7 @@ get_varp_scope(p, opt_flags)
case PV_MP: return (char_u *)&(curbuf->b_p_mp);
#endif
case PV_EP: return (char_u *)&(curbuf->b_p_ep);
+ case PV_FP: return (char_u *)&(curbuf->b_p_fp);
case PV_KP: return (char_u *)&(curbuf->b_p_kp);
case PV_PATH: return (char_u *)&(curbuf->b_p_path);
case PV_AR: return (char_u *)&(curbuf->b_p_ar);
@@ -9483,6 +9486,8 @@ get_varp(p)
/* global option with local value: use local value if it's been set */
case PV_EP: return *curbuf->b_p_ep != NUL
? (char_u *)&curbuf->b_p_ep : p->var;
+ case PV_FP: return *curbuf->b_p_fp != NUL
+ ? (char_u *)&curbuf->b_p_fp : p->var;
case PV_KP: return *curbuf->b_p_kp != NUL
? (char_u *)&curbuf->b_p_kp : p->var;
case PV_PATH: return *curbuf->b_p_path != NUL
@@ -9710,6 +9715,17 @@ get_equalprg()
return curbuf->b_p_ep;
}
+/*
+ * Get the value of 'formatprg', buffer-local or global.
+ */
+ char_u *
+get_formatprg()
+{
+ if (*curbuf->b_p_fp == NUL)
+ return p_fp;
+ return curbuf->b_p_fp;
+}
+
#if defined(FEAT_WINDOWS) || defined(PROTO)
/*
* Copy options from one window to another.
@@ -10056,6 +10072,7 @@ buf_copy_options(buf, flags)
buf->b_p_efm = empty_option;
#endif
buf->b_p_ep = empty_option;
+ buf->b_p_fp = empty_option;
buf->b_p_kp = empty_option;
buf->b_p_path = empty_option;
buf->b_p_tags = empty_option;
diff --git a/src/option.h b/src/option.h
index 434f9f3..9f333aa 100644
--- a/src/option.h
+++ b/src/option.h
@@ -958,6 +958,7 @@ enum
, BV_FF
, BV_FLP
, BV_FO
+ , BV_FP
#ifdef FEAT_AUTOCMD
, BV_FT
#endif
diff --git a/src/proto/option.pro b/src/proto/option.pro
index 15cf2b4..91318ea 100644
--- a/src/proto/option.pro
+++ b/src/proto/option.pro
@@ -34,6 +34,7 @@ void free_one_termoption __ARGS((char_u *var));
void set_term_defaults __ARGS((void));
void comp_col __ARGS((void));
char_u *get_equalprg __ARGS((void));
+char_u *get_formatprg __ARGS((void));
void win_copy_options __ARGS((win_T *wp_from, win_T *wp_to));
void copy_winopt __ARGS((winopt_T *from, winopt_T *to));
void check_win_options __ARGS((win_T *win));
diff --git a/src/structs.h b/src/structs.h
index 7b71a5f..50bedf4 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -1571,6 +1571,7 @@ struct file_buffer
char_u *b_p_efm; /* 'errorformat' local value */
#endif
char_u *b_p_ep; /* 'equalprg' local value */
+ char_u *b_p_fp; /* 'formatprg' local value */
char_u *b_p_path; /* 'path' local value */
int b_p_ar; /* 'autoread' local value */
char_u *b_p_tags; /* 'tags' local value */
--
1.7.6.295.gdcabd
Sung Pae
I was wondering if there was any feedback on the patch supplied
regarding `formatprg`:
http://groups.google.com/group/vim_dev/browse_thread/thread/7052b5523b2dc809
Is there something I could do to improve the patch, or are there
implications to this change that I have not considered?
Cheers,
Sung Pae
Tony Mechelynck
Even with a global 'formatprg', you can use buffer-local 'formatexpr'
(which overrides it). 'formatexpr' can do whatever it wants with, for
instance, values of Vim options, and, if necessary, call system() or a
!filter with any arguments it wants to. IOW I'm not convinced that the
change is necessary.
Best regards,
Tony.
--
A limerick packs laughs anatomical
Into space that is quite economical.
But the good ones I've seen
So seldom are clean,
And the clean ones so seldom are comical.
Sung Pae
### REGARDING THE DRAWBACKS OF formatexpr ###
On 13 Aug 2011, at 3:06 PM, Tony Mechelynck wrote:
> Even with a global 'formatprg', you can use buffer-local 'formatexpr'
> (which overrides it). 'formatexpr' can do whatever it wants with, for
> instance, values of Vim options, and, if necessary, call system() or a
> !filter with any arguments it wants to.
Yes, except `formatexpr` is called on every keystroke in Insert mode,
even when `formatoptions` is empty. This suggests to me that the primary
purpose of `formatexpr` is for fine-grained control over automatic
formatting, rather than to be a simple, on demand paragraph formatter
like `fmt` or `par`. [1]
Also, it is wasteful to set `formatexpr` if you wish to only use it for
`gq`. I would hesitate to hook into `InsertCharPre`, so I am hesitant to
set `formatexpr`.
Now, you can set `textwidth` to zero to avoid calling the expr on every
insertion, but then you would have to store your desired line length in
another variable.
### DEFENSE OF A LOCAL formatprg ###
> IOW I'm not convinced that the change is necessary.
If `formatprg` is meant to be set to an external program like `fmt`, is
it not reasonable that one would want different parameters (like line
length) for different file types?
The other filterprg options (`equalprg` and `grepprg` [1]) are
global-local for exactly this reason; `formatprg` is an incomplete
feature as exists now.
It is true that `filterexpr` is a superset of `formatprg`, but it comes
with a large performance and complexity cost. This discourages its use,
compared to the easily understood `formatprg`.
### REGARDING REAL-WORLD USAGE OF BOTH OPTIONS ###
Compare the usage of both options on Github:
formatprg: https://github.com/search?type=Code&language=VimL&q=formatprg
formatexpr: https://github.com/search?type=Code&language=VimL&q=formatexpr
If you dive into the search results, you'll find that people set
`formatprg` to variations of `fmt`, `par`, `astyle`, `perltidy`, `perl\
-MText::Autoformat`, `PythonTidy`, `gofmt`, `xml_pp`, and so on. A few
even try `exe "setlocal formatprg=par\ " . &textwidth`!
In comparison, the `formatexpr` search results reveal only three
different invocations:
1) setlocal formatexpr=autofmt#japanese#formatexpr()
2) setlocal formatexpr=googlecodewiki#FormatExpr()
3) setlocal formatexpr=
The third is the overwhelming majority of results, due to some common
skeleton vimrc.
Doing a google search for both options reveal the same pattern: there
are many users of `formatprg` [2], while results for `formatexpr` are
limited to references to Autofmt [3], and questions about the proper use
of `formatexpr`.
It's clear many people are very comfortable with filtering text
through an external formatter, and many choose to set `filterprg` to a
language-specific tidy program. This is another compelling reason to
implement this small change to the option's scoping rules.
Cheers,
Sung Pae
[1]: `keywordprg` and `makeprg` are also global-local, but are not
filter programs. `cscopeprg` is global-only, but that makes sense.
[2]: Perhaps due to being recently popularized by the Vimcasts episode,
"Formatting text with par".
Chiel92
Is there any progression on this subject? I'm currently writing a code format plugin, which heavily relies on this issue.
Sung Pae
> Is there any progression on this subject? I'm currently writing a code
> format plugin, which heavily relies on this issue.
formatting plugin should either use 'formatexpr' or provide it's own
functionality for the gq command. I don't know the specifics of your
plugin of course, so this is certainly not an absolute maxim.
'formatprg' is meant to be a replacement for the default internal
formatter used by the gq command (and always by gw), which is used
primarily for editing prose, not code.
That said, I still stand behind my argument for switching 'formatprg' to
a buffer-local option and have been happily using it in my own build of
Vim without issue.
Sung Pae
Chiel92
The plugin I'm talking about can be found right here: https://github.com/Chiel92/vim-autoformat
I thought that it was a good idea to utilize formatprg, since it looks like it exists for this one purpose: to format text using an external application, whatever the text maybe. Why do you think formatexpr should be used for this instead?
Sung Pae
> On Friday, January 25, 2013 3:36:21 AM UTC+1, Sung Pae wrote:
>
> > On Thu, Jan 24, 2013 at 08:45:45AM -0800, Chiel92 wrote:
> >
> > > Is there any progression on this subject? I'm currently writing a
> > > code format plugin, which heavily relies on this issue.
> >
> > While I hesitate to campaign against my own patch, I think a _code_
> > formatting plugin should either use 'formatexpr' or provide it's own
> > functionality for the gq command. I don't know the specifics of your
> > plugin of course, so this is certainly not an absolute maxim.
> >
> > 'formatprg' is meant to be a replacement for the default internal
> > formatter used by the gq command (and always by gw), which is used
> > primarily for editing prose, not code.
> >
> > That said, I still stand behind my argument for switching
> > 'formatprg' to a buffer-local option and have been happily using it
> > in my own build of Vim without issue.
>
> https://github.com/Chiel92/vim-autoformat I thought that it was a good
> idea to utilize formatprg, since it looks like it exists for this
> one purpose: to format text using an external application, whatever
> the text maybe. Why do you think formatexpr should be used for this
> instead?
unimaginative. If there exist nice external tools for formatting code
like astyle, jsbeautify, autopep8, and tidy, then it seems burdensome
to have to wrap these commands in formatexprs, or rewrite them in
vimscript.
I think I was under the impression that your plugin was trying to
achieve automatic insert mode formatting, which is only possible with
formatexpr. I'm not sure why I thought this, since you did not mention
anything of the sort.
So I retract my last response: a global-local formatprg is clearly
useful for both prose and code, and comes at the cost of a small
non-breaking change.
Tony Mechelynck's criticism is that it is superfluous. I disagree¹, but
you will have to convince him before Bram will pay any attention. :)
Sung Pae
¹ Why have formatprg at all then?
Gary Johnson
> So I retract my last response: a global-local formatprg is clearly
> useful for both prose and code, and comes at the cost of a small
> non-breaking change.
>
> Tony Mechelynck's criticism is that it is superfluous. I disagree�, but
> you will have to convince him before Bram will pay any attention. :)
>
> Sung Pae
>
> � Why have formatprg at all then?
'formatexpr' did not appear until Vim 7.
Vim seems to be moving away from relying on external programs to
expand functionality and toward having more capability built-in.
One of the reasons for this is to accommodate Windows users who
don't have access to the rich set of tools available in Unix.
Another reason is that built-in tools have access to Vim's variables
and functions and allow for better integration with Vim.
I think that 'formatexpr' offers better functionality than
'formatprg', including the ability to run external programs, which
is probably why Tony says that it's superfluous. 'formatprg' is
simpler to use in some cases and, more importantly, must be kept for
backwards compatibility.
Regards,
Gary
Justin M. Keyes
> On 2013-04-24, Sung Pae wrote:
>
> > So I retract my last response: a global-local formatprg is clearly
> > useful for both prose and code, and comes at the cost of a small
> > non-breaking change.
> >
> > Tony Mechelynck's criticism is that it is superfluous. I disagree�, but
> > you will have to convince him before Bram will pay any attention. :)
> >
> > Sung Pae
> >
> > � Why have formatprg at all then?
>
> 'formatprg' existed in Vim 6 and possibly in earlier versions;
> 'formatexpr' did not appear until Vim 7.
>
> Vim seems to be moving away from relying on external programs to
> expand functionality and toward having more capability built-in.
> One of the reasons for this is to accommodate Windows users who
> don't have access to the rich set of tools available in Unix.
> Another reason is that built-in tools have access to Vim's variables
> and functions and allow for better integration with Vim.
>
> I think that 'formatexpr' offers better functionality than
> 'formatprg', including the ability to run external programs, which
> is probably why Tony says that it's superfluous. 'formatprg' is
> simpler to use in some cases and, more importantly, must be kept for
> backwards compatibility.
Making 'formatprg' global-local is a low-risk change that reduces inconsistency and adds some convenience and flexibiity.
While flexibility of 'formatexpr' is nice, there should also be a continued push to make Vim's features more consistent where it can be done at negligible cost.
---
Justin M. Keyes