ayushtkn commented on code in PR #5838:
URL: https://github.com/apache/hive/pull/5838#discussion_r2223630277


##########
beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java:
##########
@@ -206,7 +207,7 @@ public void testVariablesForSource() {
 
   @Test
   public void testErrOutput() {
-    verifyCMD("show tables;set system:xxx=5;set 
system:yyy=${system:xxx};\nlss;",
+    verifyCMD("show tables;set system:xxx=5;set 
system:yyy=${system:xxx};\nlss;\n",

Review Comment:
   why \n kicks in here?



##########
beeline/src/java/org/apache/hive/beeline/BeeLine.java:
##########
@@ -1394,58 +1409,153 @@ private int execute(ConsoleReader reader, boolean 
exitOnError) {
     return lastExecutionResult;
   }
 
+  public String readLine(String prompt, Character mask) {
+    return readLine(getLineReader(), prompt, mask);
+  }
+
+  /**
+   * Reads a line with the given prompt and optional mask character.
+   * Starting with JLine3, an EndOfFileException is intentionally thrown upon 
reaching the end of the input stream.
+   * In interactive usage, this method returns the partial line entered before 
the exception to preserve existing
+   * behavior.
+   */
+  private String readLine(LineReader reader, String prompt, Character mask) {
+    try {
+      return reader.readLine(prompt, mask);
+    } catch (EndOfFileException eof) {
+      return eof.getPartialLine();
+    }
+  }
+
   @Override
   public void close() {
     commands.closeall(null);
+    terminalsToClose.forEach(t -> {
+      try {
+        t.close();
+      } catch (IOException e) {
+        info(String.format("Exception while closing terminal (name: %s, class: 
%s): %s", t.getName(),
+            t.getClass(), e.getMessage()));
+      }
+    });
   }
 
-  private void setupHistory() throws IOException {
+  @VisibleForTesting
+  void setupHistory() {
     if (this.history != null) {
        return;
     }
 
-    this.history = new FileHistory(new File(getOpts().getHistoryFile()));
+    this.history = new DefaultHistory();
   }
 
   private void addBeelineShutdownHook() throws IOException {
     // add shutdown hook to flush the history to history file and it also 
close all open connections
     ShutdownHookManager.addShutdownHook(getShutdownHook());
   }
 
-  public ConsoleReader initializeConsoleReader(InputStream inputStream) throws 
IOException {
-    if (inputStream != null) {
-      // ### NOTE: fix for sf.net bug 879425.
-      // Working around an issue in jline-2.1.2, see 
https://github.com/jline/jline/issues/10
-      // by appending a newline to the end of inputstream
-      InputStream inputStreamAppendedNewline = new 
SequenceInputStream(inputStream,
-          new ByteArrayInputStream((new String("\n")).getBytes()));
-      consoleReader = new ConsoleReader(inputStreamAppendedNewline, 
getErrorStream());
-      consoleReader.setCopyPasteDetection(true); // jline will detect if <tab> 
is regular character
-    } else {
-      consoleReader = new ConsoleReader(getInputStream(), getErrorStream());
-    }
+  public LineReader getFileLineReader(InputStream inputStream) throws 
IOException {
+    final LineReaderBuilder builder = LineReaderBuilder.builder();
+    defaultParser(builder);
+
+    builder.terminal(buildTerminal(prepareInputStream(inputStream)));
 
-    //disable the expandEvents for the purpose of backward compatibility
-    consoleReader.setExpandEvents(false);
+    return builder.build();
+  }
+
+  public LineReader initializeLineReader(InputStream inputStream) throws 
IOException {
+    final LineReaderBuilder builder = LineReaderBuilder.builder();
+    defaultParser(builder);
+
+    Terminal lineReaderTerminal = buildTerminal(inputStream);
+    builder.terminal(lineReaderTerminal);
 
     try {
       // now set the output for the history
       if (this.history != null) {
-        consoleReader.setHistory(this.history);
-      } else {
-        consoleReader.setHistoryEnabled(false);
+        builder.history(this.history);
+        builder.variable(LineReader.HISTORY_FILE, new 
File(getOpts().getHistoryFile()));
+        builder.variable(LineReader.HISTORY_FILE_SIZE, 
getOpts().getMaxHistoryRows());
+        // in-memory keep more data, but at least 500 entries
+        builder.variable(LineReader.HISTORY_SIZE, Math.max(500, 3 * 
getOpts().getMaxHistoryRows()));
       }
     } catch (Exception e) {
       handleException(e);
     }
 
-    if (inputStream instanceof FileInputStream || inputStream instanceof 
FSDataInputStream) {
-      // from script.. no need to load history and no need of completer, either
-      return consoleReader;
+    builder.completer(new BeeLineCompleter(this));
+    lineReader = builder.build();
+    lineReader.unsetOpt(LineReader.Option.HISTORY_TIMESTAMPED);
+    // need to disable expansion, otherwise commands (starting with "!") will 
activate history items
+    lineReader.setOpt(LineReader.Option.DISABLE_EVENT_EXPANSION);
+
+    if (this.history != null) {
+      this.history.attach(lineReader);
     }
+    return lineReader;
+  }
+
+  private void defaultParser(LineReaderBuilder builder) {
+    DefaultParser parser = new DefaultParser() {
+      private String extraNameCharacters;
+
+      // delimiters for SQL statements are any
+      // non-letter-or-number characters, except
+      // underscore and characters that are specified
+      // by the database to be valid name identifiers.
+      @Override
+      public boolean isDelimiterChar(CharSequence buffer, int pos) {
+        char c = buffer.charAt(pos);
+        if (Character.isWhitespace(c)) {
+          return true;
+        }
+        return !(Character.isLetterOrDigit(c))
+            && c != '_'
+            && extraNameCharacters().indexOf(c) == -1;
+      }
 
-    consoleReader.addCompleter(new BeeLineCompleter(this));
-    return consoleReader;
+      private String extraNameCharacters() {
+        if (extraNameCharacters != null) {
+          return extraNameCharacters;
+        }
+        try {
+          extraNameCharacters =
+              getDatabaseMetaData() == null || 
getDatabaseMetaData().getExtraNameCharacters() == null ? ""
+                  : getDatabaseMetaData().getExtraNameCharacters();
+          return extraNameCharacters;
+        } catch (NoCurrentConnectionException noCurrentConnectionException) {
+          // this is not a problem at this point, will be tried again when a 
connection is present
+          debug("No current connection found while trying to retrieve extra 
name characters.");
+          return "";
+        } catch (SQLException e) {
+          throw new RuntimeException("Error while retrieving database extra 
characters", e);
+        }
+      }
+    };
+    // In JLine3, special characters (e.g., backslash) are handled by the 
terminal by default.
+    // This is not desired: we want to send the query string to HS2 exactly as 
entered, without interpretation.
+    parser.setEscapeChars(new char[]{});
+    builder.parser(parser);
+  }
+
+  private InputStream prepareInputStream(InputStream inputStream) {
+    if (inputStream != null) {
+      inputStream = new SequenceInputStream(inputStream,
+          new ByteArrayInputStream((new String("\n")).getBytes()));
+    }
+    return inputStream;
+  }
+
+  protected Terminal buildTerminal(InputStream inputStream) throws IOException 
{
+    Terminal terminal;
+    if (inputStream != null) { // typically when there is a file script to 
read from
+      terminal = TerminalBuilder.builder().streams(inputStream, 
getErrorStream()).build();
+    } else { // no input stream, normal operation: proper behavior needs a 
system terminal
+      // system terminal can only be created with system streams

Review Comment:
   nit ``system terminal`` is coming twice here, first in the first line then 
again in the second line



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to