php interactive shell: save history on SIGINT exit

11 years ago by Dmitry Saprykin — view source — reply

unread

Php interactive shell saves commands history when you exit it using 'quit'.
But it throws all you history away when you exit using Ctrl+C. It is common
practice to save history on SIGINT exit (mysql, mongo, etc.)

I would like to implement SIGINT handler for interactive shell to save
history on Ctrl+C exit.

Threre is request on bugs.php.net for this feature
https://bugs.php.net/bug.php?id=67496.
I have created pull request https://github.com/php/php-src/pull/727 but
was advised to create RFC to discuss this change.

So could you provide some feedback.

Kind regards,
Dmitry Saprykin

11 years ago by Yasuo Ohgaki — view source — reply

unread

Hi all,

On Mon, Jul 21, 2014 at 6:12 PM, Dmitry Saprykin saprykin.dmitry@gmail.com
wrote:

Php interactive shell saves commands history when you exit it using 'quit'.
But it throws all you history away when you exit using Ctrl+C. It is common
practice to save history on SIGINT exit (mysql, mongo, etc.)

I would like to implement SIGINT handler for interactive shell to save
history on Ctrl+C exit.

Threre is request on bugs.php.net for this feature
https://bugs.php.net/bug.php?id=67496.
I have created pull request https://github.com/php/php-src/pull/727 but
was advised to create RFC to discuss this change.

So could you provide some feedback.

Isn't it nicer to treat this kind of change as simple bug fix/feature
implementation?
It's common behavior and there is no compatibility issue at all.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

11 years ago by Michael Wallner — view source — reply

unread

Hi all,

On Mon, Jul 21, 2014 at 6:12 PM, Dmitry Saprykin <
saprykin.dmitry@gmail.com>
wrote:

Php interactive shell saves commands history when you exit it using
'quit'.
But it throws all you history away when you exit using Ctrl+C. It is
common
practice to save history on SIGINT exit (mysql, mongo, etc.)

I would like to implement SIGINT handler for interactive shell to save
history on Ctrl+C exit.

Threre is request on bugs.php.net for this feature
https://bugs.php.net/bug.php?id=67496.
I have created pull request https://github.com/php/php-src/pull/727 but
was advised to create RFC to discuss this change.

So could you provide some feedback.

Isn't it nicer to treat this kind of change as simple bug fix/feature
implementation?
It's common behavior and there is no compatibility issue at all.

Yes, please do NOT create a RFC for each and every tiny feature. Just find
someone to review and eventually merge your patch!

11 years ago by Dmitry Saprykin — view source — reply

unread

Ok, I will NOT ))
God saves us from bureaucracy

Hi all,

On Mon, Jul 21, 2014 at 6:12 PM, Dmitry Saprykin <
saprykin.dmitry@gmail.com>
wrote:

Php interactive shell saves commands history when you exit it using
'quit'.
But it throws all you history away when you exit using Ctrl+C. It is
common
practice to save history on SIGINT exit (mysql, mongo, etc.)

I would like to implement SIGINT handler for interactive shell to save
history on Ctrl+C exit.

Threre is request on bugs.php.net for this feature
https://bugs.php.net/bug.php?id=67496.
I have created pull request https://github.com/php/php-src/pull/727
but
was advised to create RFC to discuss this change.

So could you provide some feedback.

Isn't it nicer to treat this kind of change as simple bug fix/feature
implementation?
It's common behavior and there is no compatibility issue at all.

Yes, please do NOT create a RFC for each and every tiny feature. Just find
someone to review and eventually merge your patch!

11 years ago by Pierre Joye — view source — reply

unread

On Mon, Jul 21, 2014 at 11:50 AM, Dmitry Saprykin
saprykin.dmitry@gmail.com wrote:

Ok, I will NOT ))
God saves us from bureaucracy

Well, Michael's view is known.

However let me explain what RFCs bring, besides bureaucracy.

clear identification of use cases, incl. edge cases
better design and design reviews
documentation, ready to be used by the doc team

I do not know how big such changes will be but I can already see a
couple of things that may require more than "works for me on my fav
linux distros" edge cases. :)

Yes, please do NOT create a RFC for each and every tiny feature. Just find
someone to review and eventually merge your patch!

--
Pierre

@pierrejoye | http://www.libgd.org

11 years ago by Bob Weinand — view source — reply

unread

Am 21.7.2014 um 12:52 schrieb Pierre Joye pierre.php@gmail.com:

On Mon, Jul 21, 2014 at 11:50 AM, Dmitry Saprykin
saprykin.dmitry@gmail.com wrote:

Ok, I will NOT ))
God saves us from bureaucracy

Well, Michael's view is known.

However let me explain what RFCs bring, besides bureaucracy.

clear identification of use cases, incl. edge cases

better design and design reviews

documentation, ready to be used by the doc team

I do not know how big such changes will be but I can already see a
couple of things that may require more than "works for me on my fav
linux distros" edge cases. :)

Yes, please do NOT create a RFC for each and every tiny feature. Just find
someone to review and eventually merge your patch!

--
Pierre

@pierrejoye | http://www.libgd.org

Here the use case is very obvious, I don't see what a RFC would make clearer here.
That it was posted to the mailing list should usually attract enough attention to quickly review such a small patch.
Here nothing needs to be documented.

People just should feel free to post it on the mailing list; if it gets controversial, then, why not, create a RFC.

Bob

11 years ago by Dmitry Saprykin — view source — reply

unread

Thank you for clarification. I'm agree with all points. But I think in this
particular case (1 file, <20 lines of code) simple review could be enough.
I'm new in php development and may be I am missing some workflow steps. If
RFC is absolutely necessary I will be happy to create it but I have no
enough wiki karma.

On Mon, Jul 21, 2014 at 11:50 AM, Dmitry Saprykin
saprykin.dmitry@gmail.com wrote:

Ok, I will NOT ))
God saves us from bureaucracy

Well, Michael's view is known.

However let me explain what RFCs bring, besides bureaucracy.

clear identification of use cases, incl. edge cases

better design and design reviews

documentation, ready to be used by the doc team

I do not know how big such changes will be but I can already see a
couple of things that may require more than "works for me on my fav
linux distros" edge cases. :)

Yes, please do NOT create a RFC for each and every tiny feature. Just
find
someone to review and eventually merge your patch!

--
Pierre

@pierrejoye | http://www.libgd.org

11 years ago by johannes@schlueters.de — view source — reply

unread

Php interactive shell saves commands history when you exit it using 'quit'.
But it throws all you history away when you exit using Ctrl+C. It is common
practice to save history on SIGINT exit (mysql, mongo, etc.)

I would like to implement SIGINT handler for interactive shell to save
history on Ctrl+C exit.

Threre is request on bugs.php.net for this feature
https://bugs.php.net/bug.php?id=67496.
I have created pull request https://github.com/php/php-src/pull/727 but
was advised to create RFC to discuss this change.

So could you provide some feedback.

I think messing with signals is not good - a script might try to catch
them, too. What we can do is to save the file each time we execute
something. (between the is valid code chack and zend_eval)

johannes

11 years ago by Dmitry Saprykin — view source — reply

unread

changed "write_history" at the end to "append_history" after
each cli_is_valid_code.
Now it is -1 line, +1 line commit and completely looks like bug fix. )

Php interactive shell saves commands history when you exit it using
'quit'.
But it throws all you history away when you exit using Ctrl+C. It is
common
practice to save history on SIGINT exit (mysql, mongo, etc.)

I would like to implement SIGINT handler for interactive shell to save
history on Ctrl+C exit.

Threre is request on bugs.php.net for this feature
https://bugs.php.net/bug.php?id=67496.
I have created pull request https://github.com/php/php-src/pull/727 but
was advised to create RFC to discuss this change.

So could you provide some feedback.

I think messing with signals is not good - a script might try to catch
them, too. What we can do is to save the file each time we execute
something. (between the is valid code chack and zend_eval)

johannes

11 years ago by johannes@schlueters.de — view source — reply

unread

changed "write_history" at the end to "append_history" after
each cli_is_valid_code.
Now it is -1 line, +1 line commit and completely looks like bug fix. )

Have you tested with libedit? - A quick grep on my system shows no
append_history() call, haven't checked further, though.

johannes

11 years ago by Tjerk Meesters — view source — reply

unread

changed "write_history" at the end to "append_history" after
each cli_is_valid_code.
Now it is -1 line, +1 line commit and completely looks like bug fix. )

Have you tested with libedit? - A quick grep on my system shows no
append_history() call, haven't checked further, though.

Yes, compilation fails with libedit (mac):

Undefined symbols for architecture x86_64:
"_append_history", referenced from:
_readline_shell_run in readline_cli.o

johannes

11 years ago by Dmitry Saprykin — view source — reply

unread

Yes, you are right. append_history is not suitable for libedit. I left
write_history call for libedit.

changed "write_history" at the end to "append_history" after
each cli_is_valid_code.
Now it is -1 line, +1 line commit and completely looks like bug fix. )

Have you tested with libedit? - A quick grep on my system shows no
append_history() call, haven't checked further, though.

johannes

11 years ago by johannes@schlueters.de — view source — reply

unread

Yes, you are right. append_history is not suitable for libedit. I left
write_history call for libedit.

Committed and pushed in a slightly modified form (variable declarations
should be on top of a block)
http://git.php.net/?p=php-src.git;a=commitdiff;h=d491b2f916d061666d9ff1cb5bdc484961b82db0

johannes