Skip to content

Conversation

@paramsiddharth
Copy link
Contributor

Closes #33: Migration to "subprocess" for executing commands

Improvements:

  • Uses subprocess.Popen instead of os.system for compilation and execution statements.
  • In each of the child processes created, the flow of stdin and stdout are controlled i. e. Both are set in subprocess.PIPE and manually controlled using obj.stdin and obj.communicate().
  • Converted all file paths into OS-agnostic paths using os.path.join. The use of '/' as a separator might cause unexpected issues in several systems; os.path takes care of that.

The problem mentioned in #31 caused by whitespaces in the directory/file names is no longer a problem after the use of subprocess. The arguments are passed as an array to subprocess.Popen, which takes care of escaping the characters in an OS-agnostic way to avoid any possible error:

...
obj = subprocess.Popen(['g++', full_filename, '-o', full_output_filename], 
                stdin=subprocess.PIPE, stdout=subprocess.PIPE)
...

The changes have been tested and Codemon successfully works in both GNU/UNIX-like (MSYS2, bash, WSL2, xterm) Windows (cmd.exe, PowerShell) environments.

@ankushbhardwxj
Copy link
Owner

@paramsiddharth - Could you reduce tabSize to 2 for all the files & use spaces instead of tabs ?

@paramsiddharth
Copy link
Contributor Author

@paramsiddharth - Could you reduce tabSize to 2 for all the files & use spaces instead of tabs ?

Sure! Let me do it.

@paramsiddharth
Copy link
Contributor Author

Done.

@paramsiddharth
Copy link
Contributor Author

PR #39 has introduced conflicts to CodemonMeta.py. Let me resolve them.

@paramsiddharth
Copy link
Contributor Author

paramsiddharth commented Dec 2, 2020

I have resolved the conflicts. Please have a look.

@paramsiddharth
Copy link
Contributor Author

paramsiddharth commented Dec 2, 2020

Turns out there are still conflicts and information lost.

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.

Migration to "subprocess" for executing commands

2 participants