Thread: psql and readline comments
Hello, please consider integrating making the usual readilne binding M-# do in psql what users expect from it: insert comment at the beginning of the line. See for patch: https://www.postgresql.org/message-id/01e0355e-381e-9732-7d4e-cbd0e6bfa710@aegee.org At https://www.postgresql.org/message-id/0afcbc3f-8c1c-c576-824a-5cc02e5a81e5@2ndquadrant.com was suggested to postpone this for PG11. Now you can integrate this on the master branch. diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c --- a/src/bin/psql/input.c +++ b/src/bin/psql/input.c @@ -356,6 +356,8 @@ initializeInput(int flags) /* these two things must be done in this order: */ initialize_readline(); rl_initialize(); + if (!strcmp("#", rl_variable_value("comment-begin"))) + rl_variable_bind("comment-begin", "--"); useHistory = true; using_history(); Regards Дилян
=?UTF-8?Q?=D0=94=D0=B8=D0=BB=D1=8F=D0=BD_?= =?UTF-8?Q?=D0=9F=D0=B0=D0=BB=D0=B0=D1=83=D0=B7=D0=BE=D0=B2?= <dpa-postgres@aegee.org>writes: > please consider integrating making the usual readilne binding M-# do in psql what users expect from it: insert comment > at the beginning of the line. See for patch: > https://www.postgresql.org/message-id/01e0355e-381e-9732-7d4e-cbd0e6bfa710@aegee.org I remain of the opinion that (1) nobody has tested the proposed patch across a range of readline and libedit versions, and (2) this would probably be better addressed by a documentation addition anyway. It's not psql's job to override user-settable readline options. regards, tom lane
Hello, when the user does not set anything, the presumption is, that the system has reasonable defaults. A reasonable default for “comment-begin” is “--”, and not the inherited default “#”. Regards Дилян On Sun, 2019-01-13 at 10:08 -0500, Tom Lane wrote: > =?UTF-8?Q?=D0=94=D0=B8=D0=BB=D1=8F=D0=BD_?= =?UTF-8?Q?=D0=9F=D0=B0=D0=BB=D0=B0=D1=83=D0=B7=D0=BE=D0=B2?= <dpa-postgres@aegee.org>writes: > > please consider integrating making the usual readilne binding M-# do in psql what users expect from it: insert comment > > at the beginning of the line. See for patch: > > https://www.postgresql.org/message-id/01e0355e-381e-9732-7d4e-cbd0e6bfa710@aegee.org > > I remain of the opinion that (1) nobody has tested the proposed patch > across a range of readline and libedit versions, and (2) this would > probably be better addressed by a documentation addition anyway. > It's not psql's job to override user-settable readline options. > > regards, tom lane
On 2019-Jan-13, Дилян Палаузов wrote: > Hello, Hello, please don't top-post. > On Sun, 2019-01-13 at 10:08 -0500, Tom Lane wrote: > > > I remain of the opinion that (1) nobody has tested the proposed patch > > across a range of readline and libedit versions, and (2) this would > > probably be better addressed by a documentation addition anyway. > > It's not psql's job to override user-settable readline options. > > when the user does not set anything, the presumption is, that the > system has reasonable defaults. A reasonable default for > “comment-begin” is “--”, and not the inherited default “#”. I agree -- it doesn't make sense to treat the insert-comment command as inserting a #, which is not a comment for psql. I use meta-# often in bash and I'm never happy to have to resort to manually prepending "--" in psql in order to make the current line a comment instead. The other suggested changes in the original thread are of course much more complicated and would require more research, but this one seems like a slam-dunk improvement. Now you may wish (as I once did) for the whole of the current input buffer to become a comment instead, but it doesn't work that way in bash either (only the current line; but then bash doesn't have multiline comments AFAIK) and I don't think psql needs to do terribly much more than that. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 14/01/2019 16:59, Alvaro Herrera wrote:
> I agree -- it doesn't make sense to treat the insert-comment command as
> inserting a #, which is not a comment for psql. I use meta-# often in
> bash and I'm never happy to have to resort to manually prepending "--"
> in psql in order to make the current line a comment instead.
I have found
rl_variable_bind("comment-begin",";");
in the clisp code, so it seems it's not unheard of to do this sort of thing.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Jan 18, 2019 at 09:18:09AM +0100, Peter Eisentraut wrote:
> On 14/01/2019 16:59, Alvaro Herrera wrote:
> > I agree -- it doesn't make sense to treat the insert-comment command as
> > inserting a #, which is not a comment for psql. I use meta-# often in
> > bash and I'm never happy to have to resort to manually prepending "--"
> > in psql in order to make the current line a comment instead.
>
> I have found
>
> rl_variable_bind("comment-begin",";");
>
> in the clisp code, so it seems it's not unheard of to do this sort of thing.
Agreed. I am not sure how someone would conditionally bind M-# to --
_only_ in psql, so it seems doing it in psql might be the logical, and
only choice. I see rl_variable_bind() referenced in libedit:
https://android.googlesource.com/platform/external/libedit/+/refs/heads/master/src/readline.c
so I think we are good in applying this to master.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
>>>>> "Bruce" == Bruce Momjian <bruce@momjian.us> writes: Bruce> Agreed. I am not sure how someone would conditionally bind M-# Bruce> to -- _only_ in psql, By doing this in .inputrc: $if psql set comment-begin "--" $endif (observe that we set rl_readline_name in order to allow this) But that shouldn't necessarily stop us doing the setting in psql by default - it just needs to be verified that it does compile against the libedit versions people are actually using, and not break anything. (On the libedit on my system, rl_variable_bind ends up trying to bind the variable as a _key_, and I have no idea what effect that would have if any. Note that libedit does not seem to bind M-# by default.) -- Andrew (irc:RhodiumToad)
On Sat, Jan 26, 2019 at 04:41:17AM +0000, Andrew Gierth wrote: > >>>>> "Bruce" == Bruce Momjian <bruce@momjian.us> writes: > > Bruce> Agreed. I am not sure how someone would conditionally bind M-# > Bruce> to -- _only_ in psql, > > By doing this in .inputrc: > > $if psql > set comment-begin "--" > $endif > > (observe that we set rl_readline_name in order to allow this) > > But that shouldn't necessarily stop us doing the setting in psql by > default - it just needs to be verified that it does compile against the > libedit versions people are actually using, and not break anything. (On > the libedit on my system, rl_variable_bind ends up trying to bind the > variable as a _key_, and I have no idea what effect that would have if > any. Note that libedit does not seem to bind M-# by default.) Uh, bind as a key? Does the patch have any effect then? If not, maybe we should just do this for libreadline. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
>>>>> "Bruce" == Bruce Momjian <bruce@momjian.us> writes: Bruce> Uh, bind as a key? Does the patch have any effect then? As I said, libedit does not bind M-# by default, and has no equivalent to readline's comment-begin variable. I have no idea what the patch would do when linked against libedit without actually trying it out (which I have not done). Bruce> If not, maybe we should just do this for libreadline. Bear in mind that for licensing reasons, some package builds build psql against libedit but then wedge readline in at runtime using LD_PRELOAD. So doing things differently at compile time based on whether it's libedit or readline will not work well with that. -- Andrew (irc:RhodiumToad)
On Tue, Jan 29, 2019 at 04:59:40AM +0000, Andrew Gierth wrote: > >>>>> "Bruce" == Bruce Momjian <bruce@momjian.us> writes: > > Bruce> Uh, bind as a key? Does the patch have any effect then? > > As I said, libedit does not bind M-# by default, and has no equivalent > to readline's comment-begin variable. I have no idea what the patch > would do when linked against libedit without actually trying it out > (which I have not done). > > Bruce> If not, maybe we should just do this for libreadline. > > Bear in mind that for licensing reasons, some package builds build psql > against libedit but then wedge readline in at runtime using LD_PRELOAD. > So doing things differently at compile time based on whether it's > libedit or readline will not work well with that. Seems like adding this would be more trouble than it is worth then. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Hello, my reading on the correspondence so far is, that: - https://www.postgresql.org/docs/current/install-requirements.html states, that libedit is compatible to libreadline, - libreadline exports the symbol rl_variable_value(), while libedit does not - both libraries export rl_variable_bind() - for licensing reasons, sometimes psql is configured and built with support for libedit, but at runtime libreadline is plugged in the dynamic linker Corollary: libreadline and libedit are not 1:1 interchangable. libreadline is compatible to libedit, but not vise- versa. What speaks against updating the documentation, stating that if psql is configured and build against libreadline, at runtime it cannot use libedit? There’s in anyway no need for such exaggerated cases. And also clarify, whether building/configuring against libedit but running against libreadline is supported. Second approach: if psql was build and configured against libedit, then set comment-begin unconditionally (or do not set it). Problematic is anyway the case, when psql is build against libreadline, but executed against libedit. Third approach: call something like: char*(psql_rl_variable_value)(const char*) = dlsym(RTLD_DEFAULT, "rl_variable_value"); //if psql_rl_variable_value is NULL, then libedit is used at runtime and not libreadline if (!psql_rl_variable_value || !strcmp("#", psql_rl_variable_value("comment-begin"))) // set comment-begin if libedit is used, or .libinput does not override the default for "comment-begin" rl_variable_bind("comment-begin", "--"); Regards Дилян On Tue, 2019-01-29 at 18:02 -0500, Bruce Momjian wrote: > On Tue, Jan 29, 2019 at 04:59:40AM +0000, Andrew Gierth wrote: > > > > > > > "Bruce" == Bruce Momjian <bruce@momjian.us> writes: > > > > Bruce> Uh, bind as a key? Does the patch have any effect then? > > > > As I said, libedit does not bind M-# by default, and has no equivalent > > to readline's comment-begin variable. I have no idea what the patch > > would do when linked against libedit without actually trying it out > > (which I have not done). > > > > Bruce> If not, maybe we should just do this for libreadline. > > > > Bear in mind that for licensing reasons, some package builds build psql > > against libedit but then wedge readline in at runtime using LD_PRELOAD. > > So doing things differently at compile time based on whether it's > > libedit or readline will not work well with that. > > Seems like adding this would be more trouble than it is worth then. >
On Thu, Jan 31, 2019 at 11:46:49AM +0000, Дилян Палаузов wrote: > Hello, > > my reading on the correspondence so far is, that: > - https://www.postgresql.org/docs/current/install-requirements.html states, that libedit is compatible to libreadline, > - libreadline exports the symbol rl_variable_value(), while libedit does not > - both libraries export rl_variable_bind() > - for licensing reasons, sometimes psql is configured and built with support for libedit, but at runtime libreadline is > plugged in the dynamic linker > > Corollary: libreadline and libedit are not 1:1 interchangable. libreadline is compatible to libedit, but not vise- > versa. > > What speaks against updating the documentation, stating that if psql is configured and build against libreadline, at > runtime it cannot use libedit? There’s in anyway no need for such exaggerated cases. And also clarify, whether > building/configuring against libedit but running against libreadline is supported. > > Second approach: if psql was build and configured against libedit, then set comment-begin unconditionally (or do not set > it). > > Problematic is anyway the case, when psql is build against libreadline, but executed against libedit. > > Third approach: call something like: > > char*(psql_rl_variable_value)(const char*) = dlsym(RTLD_DEFAULT, "rl_variable_value"); > //if psql_rl_variable_value is NULL, then libedit is used at runtime and not libreadline > if (!psql_rl_variable_value || !strcmp("#", psql_rl_variable_value("comment-begin"))) > // set comment-begin if libedit is used, or .libinput does not override the default for "comment-begin" > rl_variable_bind("comment-begin", "--"); Wow, run-time probe for the library --- interesting. I think a more simple case would be to just unconditionally set the variable, and if libedit is being used at run-time, nothing happens, but using libreadline at run-time would work. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
>>>>> "Дилян" == Дилян Палаузов <dpa-postgres@aegee.org> writes: Дилян> Hello, Дилян> my reading on the correspondence so far is, that: You missed the most important point, which is that if anyone wants to seriously propose this, they need to _actually test it_ with building against some common libedit versions to verify that it doesn't break anything. -- Andrew (irc:RhodiumToad)