-
Notifications
You must be signed in to change notification settings - Fork 10
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
Correction of BUG in subroutine parser and Support for multiple terminals #141
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have an adjusted setting to let the user choose?
src/mi2.ts
Outdated
if(this.isTerminalInstalled("xterm")){ | ||
this.createXtermTerminal(sleepVal, target); | ||
}else if(this.isTerminalInstalled("gnome-terminal")){ | ||
this.createGNOMETerminal(sleepVal, target); | ||
}else if(this.isTerminalInstalled("konsole")){ | ||
this.createKDETerminal(sleepVal, target); | ||
}else if(this.isTerminalInstalled("xfce4-terminal")){ | ||
this.createXFCETerminal(sleepVal, target); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should be a configurable gdbtty to enable choosing between the terminals, otherwise LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Simon! Something like this? "gdbtty": "xfce4"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exactly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Simon! When you have a moment, take a look and see if it's okay.
Support for multiple terminals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one! I personally would prefer two commits - one for the parser fix and one for the new feature, but that's not my repo...
I do think that the parser fix should include a change in the testsuite.
@@ -67,7 +68,7 @@ export class SourceMap { | |||
private version: string; | |||
private lineBefore: string = ""; | |||
private performLine: number = -1; // 002 - stepOver in routines with "perform" | |||
private isVersion2_2_or_3_1_1: boolean = false; | |||
private isVersion2_2_or_3_1_1: boolean = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A trailing space slipped in here - otherwise it the changes LGTM.
Hello Simon! The tests pass, but I found the issue while debugging these files 'sample.cbl', 'subsample.cbl', and 'subsubsample.cbl'. When the system switches from one file to another, for example, from 'sample' to 'subsample', the debugger goes to the last line of code. With this correction, it seems that the problem is resolved. |
I hop that @OlegKunitsyn may merge this soon. In any case and "mostly unrelated" (at least unrelated to this repo, other than your changes being likely similar): |
Hello Simon! launch.json: |
The configuration is fine - I've asked you to contribute similar code you've added to this extension (all |
Good morning, Simon! I can definitely try. It will be a pleasure. |
Support for multiple terminals – allows the Linux system to use not only XTerm but also gnome-terminal, konsole (KDE), and xfce4-terminal.
Bug in parser – in some cases, the snippet "// fix new codegen - new" was causing the parser to change the line number of the corresponding C code line after the "/* Program exit */" tag.