Skip to content

Conversation

@capkurmagati
Copy link
Contributor

Which issue does this PR close?

The cli cannot handle whitespace like

❯ \?
'\? ' is not a valid command
❯ \quiet true
'\quiet true ' is not a valid command

The PR fixes this issue by removing the consecutive whitespaces.

Closes #.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Ok(line) if line.starts_with('\\') => {
rl.add_history_entry(line.trim_end());
if let Ok(cmd) = &line[1..].parse::<Command>() {
let command = line.split_whitespace().collect::<Vec<_>>().join(" ");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I'd like to fix here is not only leading and trailing whitespaces.
Currently, the behavior is like

DataFusion CLI v5.1.0

❯ \quiet  true
'\quiet  true' is not a valid command
❯ \pset  format  csv
'\pset  format  csv' is not a valid command
❯ \pset format  csv
Execution error:  csv is not a valid format type [possible values: csv, tsv, table, json, ndjson]

Without normalizing whitespaces before parsing commands, every command need to do trim like here.
https://github.com/apache/arrow-datafusion/blob/42f8ab1e44510cea73f9eb2300d28f2fa92ac55a/datafusion-cli/src/functions.rs#L159

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i see, thanks for the context.

@houqp
Copy link
Member

houqp commented Dec 2, 2021

eventually, we will need to write a proper lexer and parser for pgcli commands :P

@houqp houqp added the bug Something isn't working label Dec 2, 2021
@houqp houqp requested a review from jimexist December 2, 2021 06:22
@alamb alamb merged commit 069449f into apache:master Dec 2, 2021
@alamb
Copy link
Contributor

alamb commented Dec 2, 2021

Thanks @capkurmagati -- I think this is a small enough fix we can iterate on it if @jimexist has some suggestions as well

@capkurmagati capkurmagati deleted the cli-handle-whiteshpaces branch December 3, 2021 07:55
unkloud pushed a commit to unkloud/datafusion that referenced this pull request Mar 23, 2025
## Which issue does this PR close?

Closes apache#1388 

## Rationale for this change

Following up on apache#1369 and apache#1386 

## What changes are included in this PR?

Updated the doc

## How are these changes tested?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants