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