Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes for debug input/output (hacked) #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rogalmic
Copy link
Collaborator

@rogalmic rogalmic commented Oct 9, 2018

No description provided.

@rogalmic rogalmic force-pushed the feature/externalDebuggerInputOutput branch from 94434f5 to 3400eae Compare October 10, 2018 06:51
@rocky rocky force-pushed the feature/externalDebuggerInputOutput branch from 3400eae to 7612204 Compare October 12, 2018 21:09
@rocky
Copy link
Collaborator

rocky commented Oct 12, 2018

This is closer to isolating the essential changes. It looks like there is still more work to do though.

Copy link
Collaborator

@rocky rocky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've changed the meaning of _Dbg_open_if_tty and set inferior-tty from the tty being read/write to being read only. Why was this necessary?

If for some reason you need a tty for reading, why not create new routines, instead change the behavior of these?

Copy link
Collaborator

@rocky rocky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm totally confused as to what you intend to do by this?

@@ -110,7 +110,7 @@ _Dbg_do_eval() {
typeset -i _Dbg_rc
if [[ -t $_Dbg_fdi ]] ; then
_Dbg_set_dol_q $_Dbg_debugged_exit_code
. $_Dbg_evalfile >&${_Dbg_fdi}
. $_Dbg_evalfile >>"$_Dbg_tty"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is appending right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually the part that is totally hacked, since i could not figure out what to do with fdi's - I just added the actual file for testing.

So the vscode extension uses 2 named pipes (debugger in, debugger out), they are expected to be separated from debugged script's in/out/err (command line options). I will check soon, maybe the appending is not needed...

@rogalmic
Copy link
Collaborator Author

You are correct, _Dbg_open_if_tty and set inferior-tty should not be touched then.

I am having problems since for example I do not get the idea of "inferior tty". The zshdb debugger has extended terminal support, I am not sure how to implement the named pipe in/out while at the same time not breaking the main functionality. In bashdb case it seemed easier for some reason...

@rocky
Copy link
Collaborator

rocky commented Oct 20, 2018

I am having problems since for example I do not get the idea of "inferior tty".

With "inferior tty" I am just trying to follow gdb. See http://visualgdb.com/gdbreference/commands/tty

There are two ways I see we could accommodate changed behavior. One way inside this repository is to add two directories new directories paralleling the command and lib directories, let's say vscommand and vslib, along with adding a new zshdb command option --vs. When that option is set, files in these new directories are read after the files in the current command and lib directories. Any functions that are defined in the new library vslib, such as _Dbg_tty() will overwrite the existing definition of _Dbg_tty() from lib.

Another possibility is just to keep a small set of patches outside and patch the code from those patches. That would work better if you wanted to patch not only for VSCode needs but also for different bash shells but down't want the bloat of the largely common sets of files that occurs among the different versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants