Bug 458884

Comment 1 Paul Webster 2015-02-01 12:12:42 EST

Just a reminder that you should be adding tests in the org.eclipse.orion.client/modules/orionode/test, and running them regularly with npm test (in the org.eclipse.orion.client/modules/orionode directory).

PW

Comment 2 Paul Webster 2015-02-01 22:28:04 EST

Please make sure that everybody sets the branch.node_git_pages.rebase = true, so that pull won't create any merge commits.

Also, you can use git config --global branch.autosetuprebase true

Then any tracking branches that are set up would get the rebase config property automatically.

PW

Comment 3 Paul Webster 2015-02-03 11:49:32 EST

(In reply to Paul Webster from comment #2)
> Please make sure that everybody sets the branch.node_git_pages.rebase =
> true, so that pull won't create any merge commits.

You'll need it for your branch, though, not node_git_pages.

Could you please comment with the github repo you're using and the branch for your feature work?

PW

Comment 4 Richard Song 2015-02-04 22:58:45 EST

Hi Paul,

The repository I'm using is https://github.com/rsong6/orion.client. The branch name is bug458884.

Also, when I do npm install inside orionode, it throws these errors. 

../src/unix/pty.cc:499:10: error: use of undeclared identifier 'openpty'
  return openpty(amaster, aslave, name, (termios *)termp, (winsize *)winp);
         ^
../src/unix/pty.cc:545:10: error: use of undeclared identifier 'forkpty'
  return forkpty(amaster, name, (termios *)termp, (winsize *)winp);

How can I resolve these?

Thanks,

Richard

Comment 5 Paul Webster 2015-02-05 08:17:37 EST

Hi Richard, thanks for the update.

The first thing I'd try is to remove the node_modules directory from orionode and then run your npm install again.

If that doesn't work, what version of linux and gcc do you have installed?

PW

Comment 6 Richard Song 2015-02-06 00:43:09 EST

Hi Paul,

I was able to get the node server up and running successfully. I documented what I did on here (https://docs.google.com/document/d/1ctGWqGXam57d6lODkD8nEvpp0iKTnpLFRDuh3tCYU40/edit?usp=sharing). I requested the owner for you to have access to this journal I will be writing for my development.

Thanks,
Richard

Comment 7 Paul Webster 2015-03-13 15:43:00 EDT

Hi Richard, some feedback:

-define(["orion/Deferred", "orion/plugin", "plugins/filePlugin/nodeFileImpl" /*node!*/, "domReady!"], function(Deferred, PluginProvider, FileServiceImpl) {
+define(["orion/Deferred", "orion/plugin", "plugins/filePlugin/fileImpl" /*node!*/, "domReady!"], function(Deferred, PluginProvider, FileServiceImpl) {

In one .js file you changed the FileServiceImpl.  Was it pointing to an implementation that wasn't there before?  In orion node, I only see the file: plugins/filePlugin/nodeFileImpl.js


searchOptions(*): if this is a prototype function (so it can be used with new) it should be SearchOptions.


I guess your next step is to implement some of the search methods.  What's one of your primary usecases?

For example, the one I'll test with is:

1) clone orion.client into the nodejs orion workspace (by hand for now).

2) I do an Edit>Global search

3) I look for operationsClient in all *.js files.

Thanks,
PW

Comment 8 Richard Song 2015-03-17 00:41:45 EDT

Hi Paul,

Sorry for the late reply. Yes, the previous implementation was pointing to the nodeFileImpl file but it was simply overriding FileServiceImpl just to disable the search capability. I needed to make it point to FileServiceImpl so that I can implement the search methods and return the appropriate results.

Sure, I'll keep that in mind.

Thanks!
Richard

Comment 9 Paul Webster 2015-03-24 14:23:21 EDT

(In reply to Richard Song from comment #8)
> Hi Paul,
> 
> Sorry for the late reply. Yes, the previous implementation was pointing to
> the nodeFileImpl file but it was simply overriding FileServiceImpl just to
> disable the search capability. I needed to make it point to FileServiceImpl
> so that I can implement the search methods and return the appropriate
> results.

Hi Richard, could you please post a code update soon, so I can provide you with some feedback.

PW

Comment 10 Richard Song 2015-03-24 21:37:19 EDT

Hi Paul,

This week is actually our school's spring break so I'll be away. I'll make sure to post a code update as soon as possible.

Thanks,

Richard

Comment 11 Richard Song 2015-04-20 01:15:16 EDT

Hi Paul,

I just pushed my code so please let me know if you would like me to change anything. It basically walks through directories and files and checks if they have content that match the search term. Also, it looks like when I put in "plugin.js" for search term and "." for filename pattern, the server builds a pattern matcher using "." but somehow is able to get the files that match plugin.js. I have been trying to debug for a while to figure how the server is able to do so but can't find it. I was wondering if you could please help me. Also, when it builds search options, it stores "plugin.js" as the filename pattern and "." as the search term, which is totally counterintuitive from the UI. I was wondering what the reasoning was behind that.


On an unrelated note, I pulled the latest code from eclipse/orion.client and eclipse/orion.server and tried running the server but I received a 404 error for plugins/npmPlugins.html, not allowing me to see the files in my workspace. I'll try taking a look at it!

Thanks,

Richard

Comment 12 Paul Webster 2015-04-20 15:55:27 EDT

(In reply to Richard Song from comment #11)
> Hi Paul,
> 
> I just pushed my code so please let me know if you would like me to change
> anything. It basically walks through directories and files and checks if
> they have content that match the search term. Also, it looks like when I put
> in "plugin.js" for search term and "." for filename pattern, the server
> builds a pattern matcher using "." but somehow is able to get the files that
> match plugin.js. I have been trying to debug for a while to figure how the
> server is able to do so but can't find it. I was wondering if you could
> please help me. Also, when it builds search options, it stores "plugin.js"
> as the filename pattern and "." as the search term, which is totally
> counterintuitive from the UI. I was wondering what the reasoning was behind
> that.

I'm not sure what you are seeing, but when I do a global search I enter gitClient as the search term and *.js as the filename pattern.  The URL my browser generates is http://172.17.0.12:8081/filesearch?sort=Path%20asc&rows=10000&start=0&q=gitClient+Name:*.js+Location:/file*

The searchOptions are correct when they hit search.js:buildSearchPattern(*), but buildSearchPattern(*) takes the searchOpts.filenamePattern (which isn't correct).

searchChild(*) does a 'var stats = fs.statSync(location);' but location is something like /orion.client and hence not valid (I get an exception here: "Error: ENOENT, no such file or directory 'orion.client/'".  It probably needs to be appended to the workspaceDir.

Also searchChild(*) needs to use the filenamePattern to filter out the files it should check for matches.




> 
> 
> On an unrelated note, I pulled the latest code from eclipse/orion.client and
> eclipse/orion.server and tried running the server but I received a 404 error
> for plugins/npmPlugins.html, not allowing me to see the files in my
> workspace. I'll try taking a look at it!


I believe npmPlugins.html has been removed, you just need to remove it from the plugin defs in the orionode node server.

PW

Comment 13 Richard Song 2015-04-23 21:40:04 EDT

> I'm not sure what you are seeing, but when I do a global search I enter
> gitClient as the search term and *.js as the filename pattern.  The URL my
> browser generates is
> http://172.17.0.12:8081/
> filesearch?sort=Path%20asc&rows=10000&start=0&q=gitClient+Name:*.js+Location:
> /file*

Sorry, this was my mistake. I accidentally switched the two parameters for my test case.

> The searchOptions are correct when they hit search.js:buildSearchPattern(*),
> but buildSearchPattern(*) takes the searchOpts.filenamePattern (which isn't
> correct).

I fixed it so that it uses the searchOpts.searchTerm to search for the files.

> searchChild(*) does a 'var stats = fs.statSync(location);' but location is
> something like /orion.client and hence not valid (I get an exception here:
> "Error: ENOENT, no such file or directory 'orion.client/'".  It probably
> needs to be appended to the workspaceDir.

I utilized the getChildren method of fileUtil.js and it seems like if the parentLocation is provided, it already joins the paths together for the location of the children. I made sure the code was scanning through the right locations.

> Also searchChild(*) needs to use the filenamePattern to filter out the files
> it should check for matches.

Yes, I will implement this over the weekend. Also, the current code doesn't use the scope parameter for the search so I will work on that right after.

Richard

Comment 14 Richard Song 2015-04-23 21:42:46 EDT

I also need to format the output and send it to client as a server response. I have been checking the responses to the request URL using the Postman app.

Comment 15 Paul Webster 2015-05-07 08:11:17 EDT

Created attachment 253287 [details]
user-profile-undefined

It looks like it found 25 files in my search for operationsClient.  But it's displaying the file names as undefined.  Maybe it's supposed to be broken up into a path and a name.

PW

Comment 16 Paul Webster 2015-05-07 08:12:00 EDT

Created attachment 253288 [details]
What user-profile.js looks like in orion

Here's what it should look like.

Comment 17 Paul Webster 2015-05-07 08:15:07 EDT

Created attachment 253289 [details]
user-profile-undefined

Ignore the attachment from comment #15

This is what it looks like with node search.

Comment 18 Paul Webster 2015-05-07 09:01:01 EDT

Code review comments:

lib/search.js:

line 2:
- copyright should read 2015

line 38:

- function searchOptions(... should be function SearchOptions since it is a type.

line 64:
- should be "var term"

line 159:
- fileLocation.match(filenamePattern) should be the first term in the expression, so you don't have to search through file contents if it's going to be filtered out anyway.

line 168:
- searchOpt.location.substring(5, searchOpt.location.length - 1); You have a magic number, '5', where did it come from?

In you next round of updates to fix the "undefined" displayed in the search results, could you please address the above changes?

I think this code is definitely heading in the right direction.  Thanks Richard.

PW

Comment 19 Richard Song 2015-05-09 14:30:49 EDT

(In reply to Paul Webster from comment #18)
> 
> In you next round of updates to fix the "undefined" displayed in the search
> results, could you please address the above changes?
> 

Oh, this was the problem that I couldn't figure out and asked you for help through Facebook. When I run Orion on the Java server, the appropriate server response is as follows. 

{
	"Directory": false,
	"LastModified": 1422397803000,
	"Length": 10111,
	"Location": "/file/rsong66-OrionContent/Orion Client/modules/orionode/lib/file.js",
	"Name": "file.js",
	"Path": "Orion Client/modules/orionode/lib/file.js"
}

I'm not exactly sure what path format the path parameter requires. I've tried out appending the workspace location to the file location, the workspace name to the file location, and /workspace/file location. Everything seems to produce "undefined". Maybe I should look into the javascript file that handles the response? If so, where should I find it?

Comment 20 Richard Song 2015-05-09 14:41:20 EDT

Never mind, I'm currently looking fileDetailRenderer.js to see how it works.

Comment 21 Paul Webster 2015-05-14 08:17:25 EDT

(In reply to Richard Song from comment #20)
> Never mind, I'm currently looking fileDetailRenderer.js to see how it works.

Any progress?

PW

Comment 22 Paul Webster 2015-05-19 10:09:54 EDT

Thanks for addressing the review comments, and the file names show up now :-)

The client says it accepts multiple file name patterns (comma-separate) but when I typed in '*.js,*.html' it no longer found the operationsClient search term.

It's OK if you don't have time to address it, please open another bug against the same Product/Component and describe the problem.

PW

Comment 23 Paul Webster 2015-06-04 14:23:52 EDT

This work has now been pushed to git.eclipse.org under the branch pwebster/bug458884

Thanks Richard.

PW

Comment 24 Paul Webster 2015-08-11 15:33:59 EDT

This has now been merged into master.

Thanks Richard.

PW