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
'
;
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;
+ }
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);
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
