Uast filtering #135

smacker · 2018-06-22T14:00:16Z

Based on: #128

Fix: #49

No protobuf decoding on FE. If we decided to go that way: we can implement it later.
Though I would experiment on dashboard first, not here.

bzz · 2018-06-26T08:08:04Z

Woks like a charm! 👏

@smacker shall we add simple progress indicator in here? Nothing fancy, a simple clue to the user that it's in progress.

On my local machine under heavy CPU load it took a while to UAST to get back and having no visual feedback in situation like this can be confusing

bzz · 2018-06-26T08:23:25Z

Few other things that seems not to be related to the UAST filtering itself were moved to #137

smacker · 2018-06-26T09:02:50Z

Yes. You are right. I missed a loader somehow, my bad. Most probably it worked too fast on my laptop :) Thanks!

carlosms

Besides the inline comments, I have two more:

Missing documentation in res-api.md
Bad layout, the main panel overflows in the bottom:

carlosms · 2018-06-26T09:54:49Z

frontend/src/components/UASTViewer.js

+      .then(uast => {
+        this.setState({ uast: transformer(uast) });
+      })
+      .catch(err => this.state({ uast: null, error: err }));


should be setState

carlosms · 2018-06-26T09:59:59Z

server/handler/query.go

@@ -116,6 +116,7 @@ func Query(db service.SQLDB) RequestProcessFunc {
 					nodes, err := service.UnmarshallUAST(val)
 					if err == nil {
 						colData[columnNames[i]] = nodes
+						colData["__uast-protobufs"] = val


Using the same name does not work when you can have multiple uast columns.

For example:

select file_path, language(file_path, blob_content) as lang, uast(blob_content, lang, '//*[@roleNumber and @roleLiteral]') as numbers, uast(blob_content, lang, '//*[@roleString and @roleLiteral]') as strings from files where lang = 'Go' limit 1

If you open the numbers column and search, you get results for the strings uast nodes.

Great catch! thank you!

carlosms · 2018-06-26T10:10:50Z

frontend/src/components/UASTViewerPane.js

+
+  let content = null;
+  if (searchResults && !searchResults.length) {
+    content = <NotFound>Nothing found</NotFound>;


should this be content = <NotFound />;?

carlosms · 2018-06-26T10:33:20Z

server/handler/uast.go

+
+		resp := &uast.Node{InternalType: "Search results"}
+
+		if req.Filter != "" {


Maybe an empty filter could be a bad request error, to simplify this part of the code.

I'm open to this change. The only problem is then we will move complexity to frontend. Please let me know it's better to have it in JS than in Go.

If you are talking about disabling the search button when the input is empty, I think it adds a nice feel to the interface and it's not that much complexity. I would make this change, but I'm also OK merging as it is. Up to you 👍

I'm talking about how to return to full UAST.
Right now you can filter than just remove the filter query and get origin UAST back.
If we disable search button we would need one more button for it :) I personally use this feature.

I think for now I would keep current behavior to be consistent with dashboard. But feel free to create an issue if you see how to improve it! Thank you.

smacker · 2018-06-26T17:12:49Z

rebased 😢 😢 😢

smacker · 2018-06-27T11:55:31Z

@bzz @carlosms please another pass

carlosms · 2018-06-27T14:39:18Z

A few comments:

UAST viewer has a loading indicator, but the CODE right panel does not. I would be good if this was consistent.
If /filter fails the whole modal content gets replaced. this means that you cannot fix an xpath query with invalid syntax, for example. I think the UAST modal should use the same error overlay as the CODE modal.
It would be better if an invalid xpath query returned an error different to 'internal server error'.

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

smacker · 2018-06-28T10:35:10Z

rebase.

not sure why I should fix code viewer in PR about uast viewer, but I did.
changed to display error similar to the one in code viewer
there is no "special" error for it. I changed 500 to bad request and return string of error. In this case: UastFilter() failed: Invalid expression. But it isn't really correct. Real internal error will be also returned as bad request. Though hopefully it's unlikely.

carlosms · 2018-06-28T11:24:25Z

there is no "special" error for it. I changed 500 to bad request and return string of error. In this case: UastFilter() failed: Invalid expression. But it isn't really correct. Real internal error will be also returned as bad request. Though hopefully it's unlikely.

Maybe we could open a request to report different this error types?

carlosms

👍 looks good to me

bzz

👏

smacker · 2018-06-29T09:47:33Z

issue to client-go: https://github.com/bblfsh/client-go/issues/65

smacker requested review from carlosms and bzz June 22, 2018 14:00

carlosms suggested changes Jun 26, 2018

View reviewed changes

carlosms mentioned this pull request Jun 27, 2018

Style for the modal UAST panel query #150

Merged

smacker added 11 commits June 28, 2018 12:09

implement filtering for uast

a4c0c41

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

return protobuf for uast

98040be

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

filter uast endpoint

7b25af9

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

code review fixes

1a8dcff

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

add loader during filtering

d395ef9

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

support multiple uast columns

cb29ad9

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

add filter endpoint documentation

4b093a0

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

fix merge conflicts

8ef17d0

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

change how error is displayed

58f28c6

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

add uast loading state to code viewer

18f0bdf

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

return filter errors as bad request

e5c728e

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>

carlosms approved these changes Jun 28, 2018

View reviewed changes

bzz approved these changes Jun 29, 2018

View reviewed changes

smacker mentioned this pull request Jun 29, 2018

Remove server input #163

Merged

smacker merged commit 4a32d62 into src-d:master Jun 29, 2018

carlosms mentioned this pull request Jul 4, 2018

Use bblfsh client-go ErrInvalidArgument #179

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uast filtering #135

Uast filtering #135

smacker commented Jun 22, 2018

bzz commented Jun 26, 2018 •

edited

Loading

bzz commented Jun 26, 2018

smacker commented Jun 26, 2018

carlosms left a comment

carlosms Jun 26, 2018

carlosms Jun 26, 2018

smacker Jun 26, 2018

carlosms Jun 26, 2018

carlosms Jun 26, 2018

smacker Jun 26, 2018

carlosms Jun 26, 2018

smacker Jun 26, 2018

smacker Jun 26, 2018

smacker commented Jun 26, 2018

smacker commented Jun 27, 2018

carlosms commented Jun 27, 2018

smacker commented Jun 28, 2018

carlosms commented Jun 28, 2018

carlosms left a comment

bzz left a comment

smacker commented Jun 29, 2018


		resp := &uast.Node{InternalType: "Search results"}

		if req.Filter != "" {

Uast filtering #135

Uast filtering #135

Conversation

smacker commented Jun 22, 2018

bzz commented Jun 26, 2018 • edited Loading

bzz commented Jun 26, 2018

smacker commented Jun 26, 2018

carlosms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smacker commented Jun 26, 2018

smacker commented Jun 27, 2018

carlosms commented Jun 27, 2018

smacker commented Jun 28, 2018

carlosms commented Jun 28, 2018

carlosms left a comment

Choose a reason for hiding this comment

bzz left a comment

Choose a reason for hiding this comment

smacker commented Jun 29, 2018

bzz commented Jun 26, 2018 •

edited

Loading