Re: add line number as prompt option to psql - Mailing list pgsql-hackers
| From | Jeevan Chalke |
|---|---|
| Subject | Re: add line number as prompt option to psql |
| Date | |
| Msg-id | CAM2+6=XWdJb_Nfk8zGCaLgc2NCQgWWWmL0Zh+-DodQT58+21VQ@mail.gmail.com Whole thread Raw |
| In response to | Re: add line number as prompt option to psql (Sawada Masahiko <sawada.mshk@gmail.com>) |
| Responses |
Re: add line number as prompt option to psql
|
| List | pgsql-hackers |
Hi,
Found new issues with latest patch:Thank you for reviewing the patch with variable cases.
I have revised the patch, and attached latest patch.
> A:> Will you please explain the idea behind these changes ?I thought wrong about adding new to tail of query_buf.
The latest patch does not change related to them.
Thanks.
> B:
I added the condition of cur_line < 0.
A.
However, this introduced new bug. As I told, when editor number of lines reaches INT_MAX it starts giving negative number. You tried overcoming this issue by adding "< 0" check. But I guess you again fumbled in setting that correctly. You are setting it to INT_MAX - 1. This enforces each new line to show line number as INT_MAX - 1 which is incorrect.
postgres=# \set PROMPT1 '%/[%l]%R%# '
postgres[1]=# \set PROMPT2 '%/[%l]%R%# '
postgres[1]=# \e
postgres[2147483646]-# limit
postgres[2147483646]-# 1;
relname
--------------
pg_statistic
(1 row)
postgres[1]=# \e
postgres[2147483646]-# =
postgres[2147483646]-# '
postgres[2147483646]'# abc
postgres[2147483646]'# '
postgres[2147483646]-# ;
relname
---------
(0 rows)
postgres[1]=# select
relname
from
pg_class
where
relname
=
'
abc
'
;
postgres=# \set PROMPT1 '%/[%l]%R%# '
postgres[1]=# \set PROMPT2 '%/[%l]%R%# '
postgres[1]=# \e
postgres[2147483646]-# limit
postgres[2147483646]-# 1;
relname
--------------
pg_statistic
(1 row)
postgres[1]=# \e
postgres[2147483646]-# =
postgres[2147483646]-# '
postgres[2147483646]'# abc
postgres[2147483646]'# '
postgres[2147483646]-# ;
relname
---------
(0 rows)
postgres[1]=# select
relname
from
pg_class
where
relname
=
'
abc
'
;
Again to mimic that, I have simply intialized newline to INT_MAX - 2.
Please don't take me wrong, but it seems that your unit testing is not enough. Above issue I discovered by doing exactly same steps I did in reviewing previous patch. If you had tested your new patch with those steps I guess you have caught that yourself.
B.
+ /* Calculate the line number */
+ if (scan_result != PSCAN_INCOMPLETE)
+ {
+ /* The one new line is always added to tail of query_buf */
+ newline = (newline != 0) ? (newline + 1) : 1;
+ cur_line += newline;
+ }
+ /* Calculate the line number */
+ if (scan_result != PSCAN_INCOMPLETE)
+ {
+ /* The one new line is always added to tail of query_buf */
+ newline = (newline != 0) ? (newline + 1) : 1;
+ cur_line += newline;
+ }
Here in above code changes, in any case you are adding 1 to newline. i.e. when it is 0 you are setting it 1 (+1) and when > 0 you are setting nl + 1 (again +1).
So why can't you simply use"
if (scan_result != PSCAN_INCOMPLETE)
cur_line += (newline + 1);
if (scan_result != PSCAN_INCOMPLETE)
cur_line += (newline + 1);
Or better, why can't you initialize newline with 1 itself and then directly assign cur_line with newline. That will eliminate above entire code block, isn't it?
Or much better, simply get rid of newline, and use cur_line itself, will this work well for you?
C. Typos:
1.
/* Count the number of new line for calculate ofline number */
Missing space between 'of' and 'line'.
However better improve that to something like (just suggestion):
"Count the number of new lines to correctly adjust current line number"
2.
/* Avoid cur_line and newline exceeds the INT_MAX */
You are saying avoid cur_line AND newline, but there is no adjustment for newline in the code following the comment.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
pgsql-hackers by date: