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

Add ProjectRun and ProjectRunList commands #354

Merged
merged 17 commits into from Dec 14, 2014

Conversation

dhleong
Copy link
Contributor

@dhleong dhleong commented Sep 27, 2014

Since #177 seems to be abandoned, this is my attempt at picking up where he left off.

:ProjectRun will execute the first-found launch config for the current project
:ProjectRunList will list all launch configs and their type for the current project

I've only used this with Android projects, but I think works very well. For other projects that run in a console inside of Eclipse, it may be possible to hook into the ILaunch to dump output, terminate launches, etc. Not sure if that is necessary for this PR or not.

The commands work well in both headless mode and headed.

@feltnerm
Copy link

Nice! I'd love to get this merged! Let me know if I can help in anyway.

(finally ditched IntelliJ for vim today, and this is the last feature I need!)

@@ -138,6 +138,7 @@ target(name: 'init'){
include(name: 'dropins/**/plugins/org.eclipse.*.jar')
include(name: 'plugins/javax.*.jar')
include(name: 'plugins/org.eclipse.*.jar')
include(name: 'plugins/org.eclipse.debug.*.jar')
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, why was this added? plugins/org.ecilpse.*.jar would match org.eclipse.debug.*.jar as well.

Copy link
Owner

Choose a reason for hiding this comment

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

Nevermind, it was added by #177 and I guess I didn't notice it there. I'll remove this line when I merge this in.

@ervandew
Copy link
Owner

For other projects that run in a console inside of Eclipse, it may be possible to hook into the ILaunch to dump output, terminate launches, etc. Not sure if that is necessary for this PR or not.

After playing around with this, I think one of the follow would need to happen:

  1. Restrict these commands to the headed version of eclimd. Or...
  2. Add some way to view that a non-gui project is in fact running, view its output, and have the ability to terminate that running process.

When I run a non-gui app from a headless instance of eclipse, aside from examining my system's process list, I have no idea what's happening with that process. So, I think releasing this feature as is would result in a lot of confusion, questions, and bug reports by users if they attempted to run a non-gui project from a headless eclimd.

@dhleong
Copy link
Contributor Author

dhleong commented Oct 19, 2014

I see, that makes sense.

I've been looking into this with Android in mind, and it might be a special
case, but I'm struggling a bit. From the ILaunch, I can do:

    IProcess[] procs = launch.getProcesses();
    if (procs.length > 0) {
      procs[0].getStreamsProxy().getOutputStreamMonitor()
    }

and possibly redirect that to a file and handle output that way (or, use
the vim remote server stuff for live updates? Sounds more compelling but I
don't have any experience with that, and seem to remember you not being
fully on board with relying on those... perhaps a flag?) but procs.length
is always 0 for my Android project. I guess that just means everything's
done when buildAndLaunch returns and there's nothing else to do? Do
normal projects that continue to run after the launch always have a
non-zero procs.length?

I think for me the coolest workflow with non-gui projects would be:

  • execute :ProjectRun
  • A new split buffer opens
  • Output from the process is asynchronously piped into the buffer, possibly
    with syntax highlighting for stderr/stdout. I'm not sure, but I suspect the
    remote server stuff would be needed here; This could be controlled by a
    flag on the command, perhaps, where the default behavior just pipes to a
    temp file that other editors can read from.
    • For vim versions without the remote command stuff, it could just run
      in the background, with a command (:ProjectRunOutput?) to open the temp
      file
  • executing :ProjectRun while there's already a running instance will
    terminate the existing one and restart (for quick run/edit/run loops)
  • future work could support :ProjectRun! to always run, even if there's
    already a running instance.
  • Closing the split via :q or whatever terminates the launch

Thoughts?

On Sun, Oct 19, 2014 at 12:05 PM, Eric Van Dewoestine <
notifications@github.com> wrote:

For other projects that run in a console inside of Eclipse, it may be
possible to hook into the ILaunch to dump output, terminate launches, etc.
Not sure if that is necessary for this PR or not.

After playing around with this, I think one of the follow would need to
happen:

  1. Restrict these commands to the headed version of eclimd. Or...
  2. Add some way to view that a non-gui project is in fact running, view
    its output, and have the ability to terminate that running process.

When I run a non-gui app from a headless instance of eclipse, aside from
examining my system's process list, I have no idea what's happening with
that process. So, I think releasing this feature as is would result in a
lot of confusion, questions, and bug reports by users if they attempted to
run a non-gui project from a headless eclimd.


Reply to this email directly or view it on GitHub
#354 (comment).

@ervandew
Copy link
Owner

On 2014-10-19 12:14:12, Daniel Leong wrote:

I see, that makes sense.

I've been looking into this with Android in mind, and it might be a special
case, but I'm struggling a bit. From the ILaunch, I can do:

    IProcess[] procs = launch.getProcesses();
    if (procs.length > 0) {
      procs[0].getStreamsProxy().getOutputStreamMonitor()
    }

and possibly redirect that to a file and handle output that way (or, use
the vim remote server stuff for live updates? Sounds more compelling but I
don't have any experience with that, and seem to remember you not being
fully on board with relying on those... perhaps a flag?)

I'm fine with using vim remote calls for now. Check out the new
VimClient class added by the new debugger support. You can see some
examples of how Kannan used it send updates to vim for the debugger
support.

but procs.length
is always 0 for my Android project. I guess that just means everything's
done when buildAndLaunch returns and there's nothing else to do? Do
normal projects that continue to run after the launch always have a
non-zero procs.length?

I'm not sure off hand, but you could create a simple class which
simply prints a line every second within an infinite loop and point a
launch configuration at that.

I think for me the coolest workflow with non-gui projects would be:

  • execute :ProjectRun
  • A new split buffer opens
  • Output from the process is asynchronously piped into the buffer, possibly
    with syntax highlighting for stderr/stdout. I'm not sure, but I suspect the
    remote server stuff would be needed here; This could be controlled by a
    flag on the command, perhaps, where the default behavior just pipes to a
    temp file that other editors can read from.

I'm fine with the initial version just assuming that the user is using
vim since the only other eclim client I've seen with real traction is
emacs-eclim, and it isn't under heavy development, so it could be
awhile before they got to this feature anyways. I need to come up with
a better way to push from eclimd for the new debugger support, so I
can update this feature accordingly when I get to that.

Fyi that all commands coming from vim do already set the '--editor'
param to 'vim'. You can access this param using:

commandLine.getValue(Options.EDITOR_OPTION)

... but like I said, I have no issues with this feature assuming the
client is vim for the first iteration.

- For vim versions without the remote command stuff, it could just run

in the background, with a command (:ProjectRunOutput?) to open the temp
file

I wouldn't spend extra time on that. If they don't have remote support
compiled in, I would just raise an error explaining that it is
required for this feature.

  • executing :ProjectRun while there's already a running instance will
    terminate the existing one and restart (for quick run/edit/run loops)
  • future work could support :ProjectRun! to always run, even if there's
    already a running instance.
  • Closing the split via :q or whatever terminates the launch

Thoughts?

This all sounds good to me.

eric

@dhleong
Copy link
Contributor Author

dhleong commented Oct 20, 2014

Awesome! I'll check it out when I find some free time

On Mon, Oct 20, 2014 at 11:05 AM, Eric Van Dewoestine <
notifications@github.com> wrote:

On 2014-10-19 12:14:12, Daniel Leong wrote:

I see, that makes sense.

I've been looking into this with Android in mind, and it might be a
special
case, but I'm struggling a bit. From the ILaunch, I can do:

IProcess[] procs = launch.getProcesses();
if (procs.length > 0) {
procs[0].getStreamsProxy().getOutputStreamMonitor()
}

and possibly redirect that to a file and handle output that way (or, use
the vim remote server stuff for live updates? Sounds more compelling but
I
don't have any experience with that, and seem to remember you not being
fully on board with relying on those... perhaps a flag?)

I'm fine with using vim remote calls for now. Check out the new
VimClient class added by the new debugger support. You can see some
examples of how Kannan used it send updates to vim for the debugger
support.

but procs.length
is always 0 for my Android project. I guess that just means everything's
done when buildAndLaunch returns and there's nothing else to do? Do
normal projects that continue to run after the launch always have a
non-zero procs.length?

I'm not sure off hand, but you could create a simple class which
simply prints a line every second within an infinite loop and point a
launch configuration at that.

I think for me the coolest workflow with non-gui projects would be:

  • execute :ProjectRun
  • A new split buffer opens
  • Output from the process is asynchronously piped into the buffer,
    possibly
    with syntax highlighting for stderr/stdout. I'm not sure, but I suspect
    the
    remote server stuff would be needed here; This could be controlled by a
    flag on the command, perhaps, where the default behavior just pipes to a
    temp file that other editors can read from.

I'm fine with the initial version just assuming that the user is using
vim since the only other eclim client I've seen with real traction is
emacs-eclim, and it isn't under heavy development, so it could be
awhile before they got to this feature anyways. I need to come up with
a better way to push from eclimd for the new debugger support, so I
can update this feature accordingly when I get to that.

Fyi that all commands coming from vim do already set the '--editor'
param to 'vim'. You can access this param using:

commandLine.getValue(Options.EDITOR_OPTION)

... but like I said, I have no issues with this feature assuming the
client is vim for the first iteration.

  • For vim versions without the remote command stuff, it could just run
    in the background, with a command (:ProjectRunOutput?) to open the temp
    file

I wouldn't spend extra time on that. If they don't have remote support
compiled in, I would just raise an error explaining that it is
required for this feature.

  • executing :ProjectRun while there's already a running instance will
    terminate the existing one and restart (for quick run/edit/run loops)
  • future work could support :ProjectRun! to always run, even if
    there's
    already a running instance.
  • Closing the split via :q or whatever terminates the launch

Thoughts?

This all sounds good to me.

eric


Reply to this email directly or view it on GitHub
#354 (comment).

If async execution is not possible, it tries to fallback on
synchronous execution. However, it will now terminate the launch
and return an error if that execution spawned long-lived processes.
For example, running an Android project does not leave any processes
behind, so it can be run synchronously even if the vim used doesn't
have remote-send support.

Still TODO: tracking the launches, opening the output buffer, handling
process death (IE: it dies on its own) and terminating processes.
Perhaps I'm missing something, but I can't seem to find a way to hook
into the IProcess's life cycle....
@dhleong
Copy link
Contributor Author

dhleong commented Oct 21, 2014

Note: Rebased off master to get VimClient. From commit notes:

If async execution is not possible, it tries to fallback on
synchronous execution. However, it will now terminate the launch
and return an error if that execution spawned long-lived processes.
For example, running an Android project does not leave any processes
behind, so it can be run synchronously even if the vim used doesn't
have remote-send support.

Still working on the async output buffer stuff, but does this sound like a decent way to handle it?

@ervandew
Copy link
Owner

On 2014-10-21 16:31:11, Daniel Leong wrote:

Note: Rebased off master to get VimClient. From commit notes:

If async execution is not possible, it tries to fallback on
synchronous execution. However, it will now terminate the launch
and return an error if that execution spawned long-lived processes.

How does it decide what is long-lived or not? Does it rely on waiting
for some pre-determined amount of time before checking if any
processes exist?

For example, running an Android project does not leave any processes
behind, so it can be run synchronously even if the vim used doesn't
have remote-send support.

Still working on the async output buffer stuff, but does this sound like a decent way to handle it?

I'd probably still recommend aborting right away on the vim side if
the user doesn't have remote-send support, which should be very rare
anyways.

eric

@dhleong
Copy link
Contributor Author

dhleong commented Oct 22, 2014

How does it decide what is long-lived or not? Does it rely on waiting
for some pre-determined amount of time before checking if any
processes exist?

It is perhaps a hack, but for these purposes "long-lived" simply means "has
any IProcess attached to the ILaunch." For Android launches there are none.

I'd probably still recommend aborting right away on the vim side if
the user doesn't have remote-send support, which should be very rare
anyways.

You would think so, but the vim that comes with OSX by default does not.
I've been using an alias to MacVim's version for my terminal use, but the
CommandExecutor of course does not pick that up since it doesn't go
through the shell, and symbolic links (for whatever reason) do not
work—running vim --serverlist on a symbolic link in /usr/bin or ~/bin to
MacVim's terminal executable shows nothing.

I don't have any strong objections to requiring remote-send from the vim
side, but the solution in the commit seemed straightforward enough if only
to support Android launches without any additional fiddling needed.

@ervandew
Copy link
Owner

On 2014-10-21 18:24:29, Daniel Leong wrote:

It is perhaps a hack, but for these purposes "long-lived" simply means "has
any IProcess attached to the ILaunch." For Android launches there are none.

I'd probably still recommend aborting right away on the vim side if
the user doesn't have remote-send support, which should be very rare
anyways.

You would think so, but the vim that comes with OSX by default does not.
I've been using an alias to MacVim's version for my terminal use, but the
CommandExecutor of course does not pick that up since it doesn't go
through the shell, and symbolic links (for whatever reason) do not
work—running vim --serverlist on a symbolic link in /usr/bin or ~/bin to
MacVim's terminal executable shows nothing.

I don't have any strong objections to requiring remote-send from the vim
side, but the solution in the commit seemed straightforward enough if only
to support Android launches without any additional fiddling needed.

So it sounds like with this solution, if another user on OSX comes
along and has a long running non-gui app and runs :ProjectRun from
macvim, then the eclimd side would kill that process because it is
deemed "long-lived" and their vim doesn't have remote support. Does
that sound correct? I think that would be another case where there
would be a bunch of bug reports soon to follow.

Perhaps the correct solution is to fix VimClient to send the remote
command to the same vim executable that the original :ProjectRun
command was executed from. The surest way to accomplish that would
probably be to have the :ProjectRun command send the path to the
currently running vim to eclimd along with the vim servername.

eric

@dhleong
Copy link
Contributor Author

dhleong commented Oct 22, 2014

It should kill it and provide a meaningful error to the user. If there is
no error then of course that would be shitty.

I considered this approach but couldn't immediately find a way for vim to
tell me which executable it is. Admittedly, I'm busy working on other
things. If you know off the top of your head, though, that'd be fantastic.

On Wed, Oct 22, 2014 at 10:41 AM, Eric Van Dewoestine <
notifications@github.com> wrote:

On 2014-10-21 18:24:29, Daniel Leong wrote:

It is perhaps a hack, but for these purposes "long-lived" simply means
"has
any IProcess attached to the ILaunch." For Android launches there are
none.

I'd probably still recommend aborting right away on the vim side if
the user doesn't have remote-send support, which should be very rare
anyways.

You would think so, but the vim that comes with OSX by default does not.
I've been using an alias to MacVim's version for my terminal use, but the
CommandExecutor of course does not pick that up since it doesn't go
through the shell, and symbolic links (for whatever reason) do not
work—running vim --serverlist on a symbolic link in /usr/bin or ~/bin
to
MacVim's terminal executable shows nothing.

I don't have any strong objections to requiring remote-send from the vim
side, but the solution in the commit seemed straightforward enough if
only
to support Android launches without any additional fiddling needed.

So it sounds like with this solution, if another user on OSX comes
along and has a long running non-gui app and runs :ProjectRun from
macvim, then the eclimd side would kill that process because it is
deemed "long-lived" and their vim doesn't have remote support. Does
that sound correct? I think that would be another case where there
would be a bunch of bug reports soon to follow.

Perhaps the correct solution is to fix VimClient to send the remote
command to the same vim executable that the original :ProjectRun
command was executed from. The surest way to accomplish that would
probably be to have the :ProjectRun command send the path to the
currently running vim to eclimd along with the vim servername.

eric


Reply to this email directly or view it on GitHub
#354 (comment).

@ervandew
Copy link
Owner

On 2014-10-22 07:55:10, Daniel Leong wrote:

It should kill it and provide a meaningful error to the user. If there is
no error then of course that would be shitty.

An error message will help, but it would still seem odd that it fails
because vim doesn't have remote support even though the user just ran
the command from a vim instance that does have remote support.

I considered this approach but couldn't immediately find a way for vim to
tell me which executable it is. Admittedly, I'm busy working on other
things. If you know off the top of your head, though, that'd be fantastic.

exepath(v:progpath) (:h v:progpath) should hopefully do the trick.

@dhleong
Copy link
Contributor Author

dhleong commented Oct 22, 2014

An error message will help, but it would still seem odd that it fails
because vim doesn't have remote support even though the user just ran
the command from a vim instance that does have remote support.

If we fix the vim executable thing with your fix below, then this would
never happen, right?

exepath(v:progpath) (:h v:progpath) should hopefully do the trick.

Awesome! :) I'll give these a shot

@dhleong
Copy link
Contributor Author

dhleong commented Oct 22, 2014

Phew. I thought MacVim didn't have v:progpath for a second, but I was just using a slightly older snapshot. I think this should work nicely

@dhleong
Copy link
Contributor Author

dhleong commented Oct 23, 2014

I believe everything is implemented as spec'd. Let me know if my handling of missing remote-send is still unsatisfactory and I'll add a client-side check.

@ervandew
Copy link
Owner

ervandew commented Nov 3, 2014

I finally got around to testing this again and I have some more comments:

  1. On my first attempt at testing this new version I opened vim (without server mode enabled) with a main class that prints to stdout every second, and ran :ProjectRun which resulted in the message: Running project '<project name>'. At that point I was left wondering what happened. It wasn't until I checked the eclimd log that I saw an error, and even then I had to look through the code to see what the error meant. I think this would be a pretty confusing experience for most users. So, should :ProjectRun fail immediately if v:servername isn't set, like :JavaDebugStart does, or is there a case where not running vim in server mode can still work (android launch configs?), in which case the eclimd side should return a useful error to the client.

  2. If I do run vim in server mode and start my project with its infinite loop, wait on the console output window, then close out vim completely (:qa), the launched java process is not killed. Adding an appropriate VimLeavePre or VimLeave autocmd should fix this issue.

  3. On one attempted I managed to trigger a NPE (possible race condition?):

    java.lang.NullPointerException
            at org.eclim.plugin.core.command.project.ProjectRunCommand$VimOutputHandler.prepare(ProjectRunCommand.java:423)
            at org.eclim.plugin.core.command.project.EclimLaunchManager.manage(EclimLaunchManager.java:131)
            at org.eclim.plugin.core.command.project.ProjectRunCommand$LaunchJob.run(ProjectRunCommand.java:484)
            at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
    
  4. In regards to the output window, one thing that could be annoying is that when appending new output, it forces the cursor to the bottom of the window. If I want to scroll up to look at some previous output, then I'm forced back down to the bottom again as soon as more output arrives. I suppose the solution here would be similar to how it's handled in a terminal emulator, and a scrollback mode needs to be added. If possible, disabling the user's ability to move the cursor in that window until scrollback mode is entered would be ideal, so that they are forced to enter scrollback mode rather than move the cursor to find something only to then have the cursor moved on them. Attempting to move the cursor could even trigger a message informing the user how to enter scrollback mode.

Aside from these issues it seems to be working great!

@dhleong
Copy link
Contributor Author

dhleong commented Nov 3, 2014

  1. Yes, Android launch configs was the case I was hoping to preserve. I thought I had handled this case, but I guess I have to go back and test this again.
  2. Ah, thanks! I assumed the BufWipeout event would get called in the right places.
  3. I'll look into this ASAP
  4. Hmmm that's a good point. I'm not familiar with scrollback mode. I think OSX's terminal stops scrolling to bottom if you scroll up manually. Perhaps something like that would be straightforward? So, if the cursor is on the last row when we're about to append, go to bottom after. If not, keep it.

@ervandew
Copy link
Owner

ervandew commented Nov 3, 2014

So, if the cursor is on the last row when we're about to append, go to bottom after. If not, keep it.

Yeah, that's a much simpler solution and the user is only a G away from resuming the normal output scrolling.

@dhleong
Copy link
Contributor Author

dhleong commented Nov 4, 2014

I believe that covers everything!

The NPE was caused by the remove expression execution timing out. I handled this case more explicitly to provide a clearer error message and increased the timeout to make it more reliable. --remote-expr seems to be quite a bit slower than --remote-send, perhaps because it can interact with the UI more consistently.

ervandew added a commit that referenced this pull request Nov 5, 2014
update the project run messaging to be more clear that vim needs to be
run in server mode.

refs #354
ervandew added a commit that referenced this pull request Nov 5, 2014
ervandew added a commit that referenced this pull request Nov 10, 2014
previous behavior was to rename the output window when the program
terminated, then on subsequent run, look for that window, delete the
buffer then create a new output window.

this caused problems, at least using console vim on linux, where the
buffer deletion would not be immediate since it occured from a remote
command call. this resulted in the server side failing to capture the
output buffer number and raising an exception. There may very well have
been some error other error on the vim side not being exposed, but it
seemed to be easier to just re-use the window. If the program status
needs to be re-added later, that can go into the statusline instead to
avoid buffer name issues.

refs #354
@ervandew
Copy link
Owner

As you've probably noticed I've created a branch and have been addressing various little issues during my testing. I also started to create a unit test for :ProjectRun but I've ran into an issue that perhaps you can help think of a solution for.

So I have a simple Main.java class like so:

public class Main
{
  public static void main(String[] args)
    throws Exception
  {
    System.out.println("Test Project");
    System.err.println("  sample error message.");
  }
}

When I go to run the project containing this file I may or may not see those 2 lines of output. This seems to be do to a race condition in the code in that the out/err listeners are attached to the process after it is already running, so it becomes a crapshoot as to whether those listeners will be added before the program has already written to out or err or not. I haven't dug into the eclipse launch api extensively yet, so I'm not sure how to solve this problem. Two potential solutions that have come to mind, but that I'm not sure how feasible they are:

  1. When adding the out/err listeners, grab any output already written by the processes and send that text to vim before sending any newly written text. I can foresee some possible issues with message ordering etc with this approach.
  2. Create some sort of dummy console for eclipse to write to directly. This would alleviate the need for the stream listeners and would probably solve some other weird issues I've seen with output sent to vim from those listeners (mainly random blank lines appearing inconsistently). Creating a custom console might also be necessary for users wanting to run console apps which require user input (stdin support).

I was all set to write the unit tests, add some docs and merge this in, but I think this issue is a show stopper. I know your main motivation for this feature is to run android apps, so if the additional overhead of writing a custom console (or whatever other solution to accommodate this) proves to be way more work than I'm sure you anticipated at the onset, an alternative to this feature request could be one specifically for org.eclim.adt which provides an :AndroidRun command to run android only launch configs. I can only imagine what other issues might arise from launch configurations for other project types, langs, etc. so this seemingly simple :ProjectRun command could continue to grow in complexity.

Any input from other users monitoring this feature request are welcome as well.

ervandew added a commit that referenced this pull request Nov 10, 2014
Note: not yet passing (may pass occasionally in theory), do to a race
condition due to std out/err stream listeners not being added to the
processes until after they have started, at which point they may have
already written to those streams.

refs #354
@dhleong
Copy link
Contributor Author

dhleong commented Nov 10, 2014

Sorry for the delay. I've been pretty busy at work and haven't been able to sit down at the AndroidXml ticket yet, either. It's still on my TODO though!

Hmm. Very good points. I don't know why, but I guess I assumed the output was buffered and that the listener would do the "right thing." I was hoping to attach a console to get the actual outputs of the build itself, but was having trouble finding a place to do that. I'll start looking into that again when I get a chance. I'm using a DebugUtil method (I think) to start the launch, so perhaps somewhere in the source of that guy we can find an answer.

Handling user input for stdin would definitely be a huge feature and is probably beyond the scope of this PR. Sure, we could add a split like the :Locate* methods and handle <enter> to submit a line—perhaps even hide this input buffer until you press i to be extra fancy—but if the project uses curses or something then that would not be sufficient, and then we're getting into reinventing something like Conque.

I agree that missing output should be a showstopper and will look into it ASAP, but I think the feature is useful enough without blocking it on stdin support.

@dhleong
Copy link
Contributor Author

dhleong commented Nov 21, 2014

Hello! It's been a while but I finally sat down and looked at this. The DebugUITools.buildAndLaunch() method is a fairly thin wrapper around ILaunchConfiguration.launch and I can't see any way to attach a console or get any output until after the launch has completed, from which I get the IProcess that in turn has a .getStreamsProxy(). Simply attaching the listeners to this proxy before attempting to prepare() the output seems to fix the problem of missing output. This proxy also has a write() method which we could use to provide input to the process.

Let me know what you think, and whether providing input is required. If so, how do you feel about my proposal for i to open the input buffer and <cr> to submit the line to the process (via the above-mentioned write())? I think the input buffer should stay until you close it manually with :q in the case that someone opens input then wants to, say, copy something from the output buffer back into the input (who knows)

@ervandew
Copy link
Owner

Unfortunately, the race condition has only been diminished, not elminated. When I manually run my test program I'll now see my output probably 90+% of the time, which is much better than before, but still potentially frustrating for the user. I can easily force the output to be lost 100% of the time by adding a sleep just before the listeners are added, so the streams are still competing to be added before the processes start up.

From the code comments:
        // By default, ProcessConsoleManager will attach a ProcessConsole
        //  that will steal any buffered output. Rude. Let's muzzle it
        //  for a second while we launch so it won't steal our output
@dhleong
Copy link
Contributor Author

dhleong commented Nov 22, 2014

Okay! After spending some time combing through eclipse source, it turns out the IStreamProxy has a getContent() method to fetch any output that was buffered before you could attach a listener. Unfortunately, the launch process allocates a ProcessConsole which rudely steals the content and flushes the buffer before we get a chance to see it.

This process is triggered by the ProcessConsoleManager attaching an ILaunchListener to the ILaunchManager as part of the normal eclipse startup stuff. I was able to grab the manager and remove its launch listeners while we do our own launch, and this finally allows us to get at that buffered content, even with a sleep() before attaching our listeners/calling getContent().

Theoretically there's a race potential where you would launch something from eclipse gui while also launching something from vim, which would result in not getting your UI console... but this seems unlikely. If we really need to support that, we could attach a temporary listener that delegates to the ProcessConsoleManager if the ILaunch's ILaunchConfiguration is not the one we're actively launching (phew).

@ervandew
Copy link
Owner

Getting very close, but I still lose the output on the very first project run per eclimd start.

  • start eclimd (headless)
  • project run: no output
  • all subsequent project runs: full output

I was testing going back and forth between the gui as well:

  • start ecilpse gui
  • start eclimd tab
  • run project from the gui: full output
  • run project from vim: full output
  • going back and forth between the 2 works fine

But if after startup I run from vim first, then just like the headless version, the first run loses all output.

  • start eclipse gui
  • start eclimd tab
  • run project from vim: no output (output goes to the gui console)
  • all subsequent project runs: full output

@dhleong
Copy link
Contributor Author

dhleong commented Nov 24, 2014

That's odd—I cannot reproduce this at all. Even re-introducing the sleep before attaching listeners, I did not lose output for the first run in vim either from headless or gui.

The only thing I can think of would be that the DebugUIPlugin is not getting initialized for you until the launch actually starts, so it would not be able to remove the launch listener. Then, on subsequent runs, it is already initialized so the removal works as expected. Still, it does not reproduce for me, so I'm not sure what to do....

@dhleong
Copy link
Contributor Author

dhleong commented Nov 24, 2014

I'm using:

Version: Luna Service Release 1 (4.4.1)
Build id: 20140925-1800

for what it's worth

@ervandew ervandew merged commit f4bd94d into ervandew:master Dec 14, 2014
ervandew added a commit that referenced this pull request Dec 14, 2014
also remove a little left over cruft.

refs #354
ervandew added a commit that referenced this pull request Dec 14, 2014
- fix :Terminate command in output buffer by setting b:eclim_project
- update TerminateAllLaunches to handle the current buffer not being in
  a project.

refs #354
ervandew added a commit that referenced this pull request Dec 14, 2014
@ervandew
Copy link
Owner

I finally got a chance this morning to dig into this and I was able to resolve the issue and get this merged in. Thank you very much for the contribution and for your patience.

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.

None yet

3 participants