datatypes.Path().edit() inconsistent behavior

[nil0x42]

Issue

Assuming that the datatypes.Path() object have an edit() method which reads the $EDITOR setting (aka session.Conf.EDITOR), it could be interesting to force this method to only return False if ui.isatty() is False.
Otherwise, someone using a curses-based editor such as vim will encounter some problems while trying to automate some phpsploit process that calls datatypes.Path().edit() even once.
Example:

phpsploit -e "set TARGET +" > /tmp/phpsploit.result

_This fails because the _set TARGET +* command tries to open the EDITOR variable's contents through the users's prefered text editor. Therefore, stdout is not a tty, so vim hangs up...*

Indeed, users of graphical editors will not suffer any problem (except that they are asked to edit something while trying to automate a phpsploit script).

Conclusion

I am still not fixed about the proper way to handle this, should we change current behavior by checking if stdin/stdout are TTYs in the edit() method, or not ???
Feel free to give your opinions!

[wapiflapi]

I'm the one who removed the TTY check in 12254a4.

I do understand the problem you're having but like you said if the check is in place graphical editors can't be used. I don't use a graphical editor my self, but my $EDITOR is set up in a way that it will start emacs in the current term if it is a TTY, or open a new terminal in which it can then start emacs, so I have the same behavior as a graphical editor when needed.

Regarding the fact that editors are asked to edit something while in automated mode that doesn't seem to be a problem to me. Actually that looks kinda useful when you want to automate something to be able to give input at certain points without having to know everything in advance in order to make it part of the script or command line.

In conclusion I'm not sure either but I'dd rather keep the current behavior of not checking. If an editor hangs when run in an environment it doesn't like isn't that a bug with the editor? I would expect it to exit with a failure, in which case phpsploit's edit() could return the failure to edit as well. But putting the isatty() check prevents one to use editors or setups that do handle the case.

Ps: As this is marked "invalid" I'm not sure if you still wanted input on it but at least this will serve as a record for why the check was removed.

[nil0x42]

Thanks for this interesting feedback, especially the part about allowing user to be able to handle special cases by itself.
I'm ok too to keep edit()'s behavior as is works now