You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by cgivre <gi...@git.apache.org> on 2017/08/07 04:06:01 UTC

[GitHub] drill pull request #897: Drill-5703 Added Syntax Highlighting and Limited Au...

GitHub user cgivre opened a pull request:

    https://github.com/apache/drill/pull/897

    Drill-5703 Added Syntax Highlighting and Limited Autocomplete on Query Page

    This is a clean PR for #893 . This PR replaces the textarea on the query page with the Ace Code editor. (https://ace.c9.io)  

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cgivre/drill ui

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/897.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #897
    
----
commit 309ef86bed73b39d39b2829df95c1b60afd9874d
Author: cgivre <cg...@gmail.com>
Date:   2017-08-07T03:58:43Z

    Added Code Editor on Query Page

commit ad30bbddcec49f32f9b022e0b833b9f0165c5df2
Author: cgivre <cg...@gmail.com>
Date:   2017-08-07T04:03:59Z

    Removed extraneous javasript files from Ace build

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:

    https://github.com/apache/drill/pull/897
  
    @cgivre can we close this PR if the #1043 met the objectives?


---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:

    https://github.com/apache/drill/pull/897
  
    @parthchandra I remember once I heard that number of rows sent to Web UI in Drill is limited. From the quick glance I did not see such limitation in code. May I assume it's not true?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by cgivre <gi...@git.apache.org>.
Github user cgivre commented on the issue:

    https://github.com/apache/drill/pull/897
  
    Yes, it can be closed.  Nice work on the syntax highlighting!
    
    > On Jan 29, 2018, at 16:46, Kunal Khatua <no...@github.com> wrote:
    > 
    > @cgivre <https://github.com/cgivre> can we close this PR if the #1043 <https://github.com/apache/drill/pull/1043> met the objectives?
    > 
    > —
    > You are receiving this because you were mentioned.
    > Reply to this email directly, view it on GitHub <https://github.com/apache/drill/pull/897#issuecomment-361397564>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFQfvsiUGAdr4o4r3V4cQQiUaQclNr13ks5tPjxFgaJpZM4Ou-Rh>.
    > 
    



---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:

    https://github.com/apache/drill/pull/897
  
    Thanks, @cgivre ! I don't have permissions to close the PR. Perhaps you can close it instead.


---

[GitHub] drill pull request #897: Drill-5703 Added Syntax Highlighting and Limited Au...

Posted by cgivre <gi...@git.apache.org>.
Github user cgivre closed the pull request at:

    https://github.com/apache/drill/pull/897


---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:

    https://github.com/apache/drill/pull/897
  
    @cgivre  I might be digressing, but I'm wondering if the formatter is editable. So, for example, functions or keywords that are supported in Drill specifically could be highlighted correctly, while unknown functions/keywords are marked as an error.
    
    @parthchandra I agree that we need to support pagination, but I think the SQL syntax highlighting needn't be tied with that feature.  


---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:

    https://github.com/apache/drill/pull/897
  
    Well, I did not mean we should not enhance Web UI. Storage plugin page enhancement would be definitely a huge plus. It's just I have a feeling that we might consider splitting admin part and query running part.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on the issue:

    https://github.com/apache/drill/pull/897
  
    In general, I'm not in favor of including large javascript files into the code base, except where the JS might be getting deprecated or is not freely downloadable from the web. Alternatively, we could download the required js files as part of the build step.
    See this commit (https://github.com/apache/drill/pull/891/commits/3779e8337692edcdbf0d2f1a1e0afbf1b6e5f255)
    
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on the issue:

    https://github.com/apache/drill/pull/897
  
    I also feel we need to fix the pagination of results before we make improvements to the editor. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by cgivre <gi...@git.apache.org>.
Github user cgivre commented on the issue:

    https://github.com/apache/drill/pull/897
  
    @kkhatua I think the formatter is editable.  Truthfully, my main goal is to implement JSON syntax highlighting on the plugin config pages.  I've spent a decent amount of time debugging JSON because of a missed comma or something.


---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:

    https://github.com/apache/drill/pull/897
  
    IMHO, Drill Web UI servers more as admin console where query profiles can be seen, storage plugins updated as well as system options etc. We might think of disabling Query page allowing users run queries using REST API, sqlline, JDBC, ODBC only and do not spend efforts on enhancing Query page.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by cgivre <gi...@git.apache.org>.
Github user cgivre commented on the issue:

    https://github.com/apache/drill/pull/897
  
    Hi Arina, 
    I’d also like to add syntax highlighting to the storage plugin page as well.  IMHO, it’s really easy to make a small mistake, missing a comma or quote or something, and it can be really difficult to find. 
    
    I’d also respectfully disagree that we shouldn’t improve the UI.  IMHO, the Drill UI doesn’t need to be super fancy, but with a few enhancements, it could be made to be much more useful than it currently is and hopefully this could help improve adoption. 
    
    — C
    
    > On Aug 10, 2017, at 07:14, Arina Ielchiieva <no...@github.com> wrote:
    > 
    > IMHO, Drill Web UI servers more as admin console where query profiles can be seen, storage plugins updated as well as system options etc. We might think of disabling Query page allowing users run queries using REST API, sqlline, JDBC, ODBC only and do not spend efforts on enhancing Query page.
    > 
    > —
    > You are receiving this because you authored the thread.
    > Reply to this email directly, view it on GitHub <https://github.com/apache/drill/pull/897#issuecomment-321522798>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFQfvo1MY2eV9OhherU7yRcp8LNiuE7-ks5sWuX-gaJpZM4Ou-Rh>.
    > 
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on the issue:

    https://github.com/apache/drill/pull/897
  
    I might be mistaken then. But that would make things worse. The UI will wait to return all the records from a query that could return a million records. 
    @cgivre that is what I mean by pagination. A query returning more than a screenful of records should return results page by page, and it should be cancelable. Without that, the query UI is a dangerous place.
    Take a look at SQLPad (http://rickbergfalk.github.io/sqlpad/installation-and-administration/) maybe we can integrate that for the query UI?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #897: Drill-5703 Added Syntax Highlighting and Limited Autocompl...

Posted by cgivre <gi...@git.apache.org>.
Github user cgivre commented on the issue:

    https://github.com/apache/drill/pull/897
  
    @parthchandra What do you want to fix in the pagination?  IMHO, the query page and the storage plugin editor both are in dire need of syntax highlighting.  I was going to work on that next.  In the meantime, I can include the JS files in the build.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---