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

Commands not in sync when no system.describe in builtin RPC #475

Closed
bilogic opened this issue Mar 17, 2019 · 18 comments
Closed

Commands not in sync when no system.describe in builtin RPC #475

bilogic opened this issue Mar 17, 2019 · 18 comments
Labels
Bug resolved if issue is resolved, it will be open until merge with master

Comments

@bilogic
Copy link

bilogic commented Mar 17, 2019

I have question related to jQuery Terminal

I used fire several exec() from inside onInit() in v0.9.3, it worked correctly.
My output was always:

Output A

prompt1>command1
result1
prompt2>command2
result2
prompt3>command3
result3
prompt4>command4
result4
...

On dev 2.2.1 using exec(), I'm getting

Output B

prompt1>command1
prompt1>command2
result1
prompt1>command3
result2
prompt1>command4
result3
result4
prompt1>

I switch to using execHash and it worked correctly, i.e. output A

My question is, what is the recommended way to use exec() so that the output is correct, i.e. output A? Thank you.

@jcubic
Copy link
Owner

jcubic commented Mar 17, 2019

Is you commands async or they are just echo something?

if you use multiple execs use array:

$.terminal.active().exec(['a', 'b', 'a', 'b'])

if your commands are async (ajax or setTimeout etc.) you need to use pause()/resume() (main reason for those functions is not to hide command line, but to have proper timing with exec).

Example:

$('???').terminal(function(cmd) {
   term.pause();
   setTimeout(() => this.echo('your commands is ' + cmd).resume(), 1000);
}, {
  onInit: function() {
      this.exec(['command1', 'command2', 'command3', 'command4']);
  }
}

this will also work if you call exec one by one:

     onInit: function() {
         this.exec('command1');
         this.exec('command2');
         this.exec('command3');
     },

NOTE: you can't call this.exec('cmd1').exec('cmd2'); because exec return a promise that resolve when you call resume.

Instead of resume you can also return a promise from function, but it need to be resolved to undefined otherwise the value will be echoed to terminal, unless this is something you want.

        .terminal(function(cmd) {
             if (cmd == 'x') {
                 return $.get('exec.html').then((data) => {
                     this.echo(data.split('\n')[0]); // no return so promise will resolve to undefined
                 });
             } else {
                 this.pause();
                 setTimeout(() => this.echo(cmd).resume(), 500);
             }
         }, {
             onInit: function() {
                 this.exec(['x', 'cmd1', 'cmd2']);
             }
         });

The promise is very flexible it only need to be object that have then function, so it work with native Promises or jQuery defereds.

@bilogic
Copy link
Author

bilogic commented Mar 17, 2019

Hi,

  1. I use a URL string, i.e. $('#pjax-main').terminal("{{ url.rpc }}", options);
  2. Both the prompt and result are returned via RPC, I suppose that means they are async?

@bilogic
Copy link
Author

bilogic commented Mar 17, 2019

Actually, execHash() does exactly and correctly what I need, but exec() behaves very differently. Also, I don't see execHash() using any value for setTimeout().

Is there a way to run commands using exec() but have the correct output like execHash()?

@jcubic
Copy link
Owner

jcubic commented Mar 17, 2019

This should work the same, do you the code somewhere online, where I can test this?

@bilogic
Copy link
Author

bilogic commented Mar 17, 2019

Hi,

I have emailed you the links.

@jcubic
Copy link
Owner

jcubic commented Mar 17, 2019

Not sure what the problem is. Maybe because you don't have system.describe method and different code is executed for rpc interpreter. Will investigate, this seems to be a bug.

@jcubic
Copy link
Owner

jcubic commented Mar 17, 2019

yes, the problem is system.describe if it's not there commands are not in sync. Same happen if you use ignoreSystemDescribe: true (which you should use if you don't have system.describe so no extra request at the beginning will happen).

@jcubic jcubic changed the title Recommended way to use exec() Commands not in sync when no system.describe in build in RPC Mar 17, 2019
@jcubic jcubic added Bug and removed question labels Mar 17, 2019
@bilogic
Copy link
Author

bilogic commented Mar 17, 2019

Hi,

Thanks! I did add describe: false, in the options earlier, but jquery.terminal.js it threw an exception and I stopped pursuing. https://terminal.jcubic.pl/api_reference.php indicates that ignoreSystemDescribe is replaced since 1.5

@jcubic
Copy link
Owner

jcubic commented Mar 17, 2019

Hm, the original option was not removed to not break existing code (that's why major version was not changed) will need to investigate describe: false then.

@jcubic jcubic changed the title Commands not in sync when no system.describe in build in RPC Commands not in sync when no system.describe in builtin RPC Mar 17, 2019
@bilogic
Copy link
Author

bilogic commented Mar 18, 2019

Hi,

2 related questions:

  1. Should I open an issue for describe: false throwing exceptions?
  2. What is the purpose of enabling system.describe? i.e. benefits of having it etc

Thank you.

@jcubic
Copy link
Owner

jcubic commented Mar 18, 2019

If you have system.describe, terminal know RPC methods and can create completion for you. So you can use completion: true.

@jcubic
Copy link
Owner

jcubic commented Mar 18, 2019

Both errors should be fixed.

@bilogic
Copy link
Author

bilogic commented Mar 20, 2019

Hi,

If I bring up chrome's developer tools and Disable cache is checked, commands intermittently go out of sync. This is done with describe: false

If the developer tools is closed (implying Disable cache is unchecked), commands are in sync.

@bilogic
Copy link
Author

bilogic commented Mar 20, 2019

Just to add, it seems very hard to replicate out-of-sync in a normal Chrome tab even with DevTools + Disable cache checked.

If I load the same link in a fancybox's iframe, out-of-sync happens more frequently.
From initial observation, it seems to happen if first few prompt/RPC completes before the iframe is shown on screen.

Any ideas? Thank you.

@jcubic
Copy link
Owner

jcubic commented Mar 20, 2019

Will check. I can also add option to exec only when terminal is visible, but this will be workaround, nevertheless it will be useful option.

@jcubic
Copy link
Owner

jcubic commented Mar 24, 2019

I cannot reproduce, can you provide a demo? the last url/user/passwd don't work:

I was testing using this data uri:

data:text/html,<style>body{margin: 0} iframe { display: none} </style><iframe style="width:100%; height: 100%" src="http://localhost/projects/jcubic/terminal/test/manual/exec.html"></iframe><script>window.onload = () => setTimeout(() => document.querySelector('iframe').style.display = 'block', 3000)</script>

@jcubic jcubic added the resolved if issue is resolved, it will be open until merge with master label Apr 5, 2019
@jcubic
Copy link
Owner

jcubic commented Apr 5, 2019

no reply after almost 2 weeks, it should be fixed not possible to reproduce in iframe for new issue.

@bilogic
Copy link
Author

bilogic commented Apr 5, 2019

Hi,

Sorry, had other matters on hand. Will update this issue if I find a reliable method to reproduce it. Let's close this for now. Thank you.

@jcubic jcubic closed this as completed Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug resolved if issue is resolved, it will be open until merge with master
Projects
None yet
Development

No branches or pull requests

2 participants