Pig
  1. Pig
  2. PIG-2706

Add clear to list of grunt commands

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.11, 0.10.1
    • Fix Version/s: 0.11
    • Component/s: grunt
    • Labels:
      None

      Description

      grunt should be able to clear screen (similar to unix shell) on a command, we can call it "clear". After typing in several lines in the interactive mode, it would be useful to be able to clear screen.

      1. PIG-2706-6.patch
        8 kB
        Gianmarco De Francisci Morales
      2. PIG-2706-5
        8 kB
        Allan Avendaño
      3. PIG-2706-4
        6 kB
        Allan Avendaño
      4. PIG-2706-3
        2 kB
        Allan Avendaño
      5. PIG-2706-2
        2 kB
        Allan Avendaño
      6. PIG-2706
        2 kB
        Allan Avendaño

        Activity

        Hide
        Allan Avendaño added a comment -

        I think that it could be implemented with some break lines. Is it the expected functionality?

        Show
        Allan Avendaño added a comment - I think that it could be implemented with some break lines. Is it the expected functionality?
        Hide
        Prashant Kommireddi added a comment -

        Thanks for looking into this Allan.

        I was thinking more of what happens on a Unix shell when you hit "clear". With your patch, the cursor might still be at the end of the grunt window after 14 new lines.

        Show
        Prashant Kommireddi added a comment - Thanks for looking into this Allan. I was thinking more of what happens on a Unix shell when you hit "clear". With your patch, the cursor might still be at the end of the grunt window after 14 new lines.
        Hide
        Daniel Dai added a comment -

        I think what "clear" does should be clearing the script cache. I feel wired if it only prints out a bunch of new lines. Thoughts?

        Show
        Daniel Dai added a comment - I think what "clear" does should be clearing the script cache. I feel wired if it only prints out a bunch of new lines. Thoughts?
        Hide
        Prashant Kommireddi added a comment -

        I agree, printing new lines is not right. However, would clearing the script cache clear out the history? The goal here should be just so that its easy for users to clear out the text (pig statements) on their terminal, and be able to start fresh from the top of the terminal (just as with "clear" command on unix shell).

        Show
        Prashant Kommireddi added a comment - I agree, printing new lines is not right. However, would clearing the script cache clear out the history? The goal here should be just so that its easy for users to clear out the text (pig statements) on their terminal, and be able to start fresh from the top of the terminal (just as with "clear" command on unix shell).
        Hide
        Allan Avendaño added a comment -

        I found that on this way it is possible to clear and to set the cursor to 0,0 (like Unix shell). There is just one problem, it works only on a terminal.
        I have to look for a way to make it works on Eclipse console.

        Show
        Allan Avendaño added a comment - I found that on this way it is possible to clear and to set the cursor to 0,0 (like Unix shell). There is just one problem, it works only on a terminal. I have to look for a way to make it works on Eclipse console.
        Hide
        Gianmarco De Francisci Morales added a comment -

        I think we can be happy with terminal only, especially if this is the ANSI standard way of clearing the console.

        If the rest of the community agrees I would commit it once tests pass.

        Show
        Gianmarco De Francisci Morales added a comment - I think we can be happy with terminal only, especially if this is the ANSI standard way of clearing the console. If the rest of the community agrees I would commit it once tests pass.
        Hide
        Prashant Kommireddi added a comment -

        Agree with you Gianmarco. I still need to test the patch, will report back once I am done with that.

        Thanks Allan again for the work.

        Show
        Prashant Kommireddi added a comment - Agree with you Gianmarco. I still need to test the patch, will report back once I am done with that. Thanks Allan again for the work.
        Hide
        Prashant Kommireddi added a comment -

        Hi Allan, patch looks good for the most part. Just a couple of minor things

        1. There seem to be a few extra newlines in printClear()

            @Override
        	protected void printClear() {
            	
            	System.out.print(((char) 27)+"[2J");
            	System.out.print(((char) 27)+"[;H");
            	
        	}
        

        2. Same at the end of GruntParser.java

             private int mNumSucceededJobs;
             private FsShell shell;
             private boolean mScriptIllustrate;
        -    
        +
        +       
        }
        
        Show
        Prashant Kommireddi added a comment - Hi Allan, patch looks good for the most part. Just a couple of minor things 1. There seem to be a few extra newlines in printClear() @Override protected void printClear() { System .out.print((( char ) 27)+ "[2J" ); System .out.print((( char ) 27)+ "[;H" ); } 2. Same at the end of GruntParser.java private int mNumSucceededJobs; private FsShell shell; private boolean mScriptIllustrate; - + + }
        Hide
        Allan Avendaño added a comment -

        Thanks for the review. Minor changes done.

        Show
        Allan Avendaño added a comment - Thanks for the review. Minor changes done.
        Hide
        Prashant Kommireddi added a comment -

        A minor one again, indentation is off a bit with the method declaration (needs to shift left).

             @Override
        +	protected void printClear() {
        +    	System.out.print(((char) 27)+"[2J");
        +    	System.out.print(((char) 27)+"[;H");
        +    }
        
        Show
        Prashant Kommireddi added a comment - A minor one again, indentation is off a bit with the method declaration (needs to shift left). @Override + protected void printClear() { + System .out.print((( char ) 27)+ "[2J" ); + System .out.print((( char ) 27)+ "[;H" ); + }
        Hide
        Daniel Dai added a comment -

        Can we also find a way for Windows?

        Show
        Daniel Dai added a comment - Can we also find a way for Windows?
        Hide
        Prashant Kommireddi added a comment -

        Allan, were you planning to continue working on this?

        Show
        Prashant Kommireddi added a comment - Allan, were you planning to continue working on this?
        Hide
        Allan Avendaño added a comment -

        Really sorry for the delay, I was focused on GSOC implementation.

        While looking for an option on Windows, I found that Java doesn't provide a way to control the prompt. But, there exists a library: Jansi, which provides a simple way to clear the prompt/terminal. I tested on Windows and Mac, on a different project, and works properly.
        Now, I have some problems on integrating the library while generating the pig.jar. How could I deal with it?

        Show
        Allan Avendaño added a comment - Really sorry for the delay, I was focused on GSOC implementation. While looking for an option on Windows, I found that Java doesn't provide a way to control the prompt. But, there exists a library: Jansi, which provides a simple way to clear the prompt/terminal. I tested on Windows and Mac, on a different project, and works properly. Now, I have some problems on integrating the library while generating the pig.jar. How could I deal with it?
        Hide
        Allan Avendaño added a comment -

        I forgot to add the url of Jansi: http://jansi.fusesource.org/

        Show
        Allan Avendaño added a comment - I forgot to add the url of Jansi: http://jansi.fusesource.org/
        Hide
        Gianmarco De Francisci Morales added a comment -

        To use a library in Pig you need to create a new dependency in ivy.xml.
        It should go something like this:

        <dependency org="org.fusesource.jansi" name="jansi" rev="1.9" conf="compile->master"/>
        
        Show
        Gianmarco De Francisci Morales added a comment - To use a library in Pig you need to create a new dependency in ivy.xml. It should go something like this: <dependency org= "org.fusesource.jansi" name= "jansi" rev= "1.9" conf= "compile->master" />
        Hide
        Allan Avendaño added a comment -

        As usual, thanks Gianmarco for the hint!

        I tested the code against Windows 7, Xubuntu and OSX Lion 10.7, and works properly. I attached the patch for your comments.

        Show
        Allan Avendaño added a comment - As usual, thanks Gianmarco for the hint! I tested the code against Windows 7, Xubuntu and OSX Lion 10.7, and works properly. I attached the patch for your comments.
        Hide
        Gianmarco De Francisci Morales added a comment -

        Patch looks good.
        A couple of minor things:

        We use spaces not tabs, can you regenerate the patch with proper formatting?
        https://cwiki.apache.org/confluence/display/PIG/HowToContribute#HowToContribute-MakingChanges

        Do we need to separate the cases Windows/non-Windows in GruntParser.java? Isn't Jansi cross platform?

        I tested it manually and it works fine. I guess it is very hard to test it automatically so we can do without tests.

        Show
        Gianmarco De Francisci Morales added a comment - Patch looks good. A couple of minor things: We use spaces not tabs, can you regenerate the patch with proper formatting? https://cwiki.apache.org/confluence/display/PIG/HowToContribute#HowToContribute-MakingChanges Do we need to separate the cases Windows/non-Windows in GruntParser.java? Isn't Jansi cross platform? I tested it manually and it works fine. I guess it is very hard to test it automatically so we can do without tests.
        Hide
        Allan Avendaño added a comment -

        code formatted and cleaning prompt is done within Jansi

        Show
        Allan Avendaño added a comment - code formatted and cleaning prompt is done within Jansi
        Hide
        Gianmarco De Francisci Morales added a comment -

        +1 minus a small change:
        I could not see the need of adding a resolver to ivy as jansi is already in maven2.
        I removed the added resolver and the patch still works fine.

        Thanks for you contribution, Allan!

        Show
        Gianmarco De Francisci Morales added a comment - +1 minus a small change: I could not see the need of adding a resolver to ivy as jansi is already in maven2. I removed the added resolver and the patch still works fine. Thanks for you contribution, Allan!
        Hide
        Gianmarco De Francisci Morales added a comment -

        The final patch I committed.

        Show
        Gianmarco De Francisci Morales added a comment - The final patch I committed.
        Hide
        Prashant Kommireddi added a comment -

        Thanks Allan for the contribution.

        Show
        Prashant Kommireddi added a comment - Thanks Allan for the contribution.

          People

          • Assignee:
            Allan Avendaño
            Reporter:
            Prashant Kommireddi
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development