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

Can cooperate with Atom Terminals (allows inputs) #2337

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

aminya
Copy link
Member

@aminya aminya commented Dec 29, 2020

To solve a common 'problem' seen by users regarding inputs not working, send commands from Script to Terminus to run rather than Atom's Buffered Process. Code is run in Terminus' internal terminal window and allows for Input (If Terminus not installed, returns to default behavior).

Originally posted by @Kacarott in #2237

lib/runtime.js Outdated

// Will cooperate with Terminus to allow for inputs, if user has installed.
try {
var terminus = require('../../terminus/lib/terminus.js').provideTerminus();
Copy link
Member

@UziTech UziTech Jan 2, 2021

Choose a reason for hiding this comment

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

we should consume the terminusTerminal service to get the terminal.

Copy link
Member

Choose a reason for hiding this comment

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

for that matter there should be a standard terminal service that all terminals can provide to allow running in any terminal package.

Copy link
Member

Choose a reason for hiding this comment

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

I have started creating PRs for terminal packages to provide the terminal service.

bus-stop/x-terminal#236
bus-stop/terminus#177
more to come...

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you make a PR fixing this?

Choose a reason for hiding this comment

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

Is this still in works? It doesn't seem to be implemented in the actual release

"^1.1.0": "consumeTerminal"
}
},
"terminal": {
Copy link
Member

Choose a reason for hiding this comment

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

This allow almost any terminal including old ones that haven't been updated in a while. If we only want currently maintained terminals we can just consume terminal.

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't hurt to support them if this doesn't break other things. Regarding package curation, I have some good plans which will solve many issues.

@aminya aminya changed the title Can cooperate with Terminus (allows inputs) Can cooperate with Atom Terminals (allows inputs) Mar 21, 2021
@aminya
Copy link
Member Author

aminya commented Mar 21, 2021

Merging requires squashing and using feat: in the beginning to trigger the release.

Copy link
Member Author

@aminya aminya left a comment

Choose a reason for hiding this comment

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

I get this error

❯ printf "\33c\e[3J" && cmd "/c C:\Users\aminy\Documents\GitHub\JavaScript\@atom-ide-community\atom-script\node_modules\.bin\babel.cmd --filename C:\Users
\aminy\Documents\GitHub\JavaScript\@atom-ide-community\atom-script\examples\modern.js --config-file C:\Users\aminy\Documents\GitHub\JavaScript\@atom-ide-c
ommunity\atom-script\lib\grammars\babel.config.js < C:\Users\aminy\Documents\GitHub\JavaScript\@atom-ide-community\atom-script\examples\modern.js| node"
printf: The term 'printf' is not recognized as a name of a cmdlet, function, script file, or executable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

lib/runtime.js Outdated Show resolved Hide resolved
Copy link
Member Author

@aminya aminya left a comment

Choose a reason for hiding this comment

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

If a terminal is not already open nothing happens. We should fall back to the original method in these cases

Co-authored-by: Tony Brix <tony@brix.ninja>
@aminya
Copy link
Member Author

aminya commented Mar 21, 2021

If a terminal is not already open nothing happens. We should fall back to the original method in these cases

It seems it is fixed. But not sure why it was happening. I hope it isn't a random bug in the terminal service.

Copy link
Member Author

@aminya aminya left a comment

Choose a reason for hiding this comment

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

The terminal package doesn't scroll to the last output if you scroll upwards intentionally.

for (let i = 0; i < commandContext.args.length; i++) {
command += ` "${commandContext.args[i]}"`
}
terminal.run([command])
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a way to make sure that this has successfully run?

@aminya aminya closed this Jul 22, 2021
@aminya aminya closed this Jul 22, 2021
@aminya aminya reopened this Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants