Skip to content

The value of !set command support multi line#93

Closed
xiedeyantu wants to merge 3 commits intohydromatic:mainfrom
xiedeyantu:multi-line
Closed

The value of !set command support multi line#93
xiedeyantu wants to merge 3 commits intohydromatic:mainfrom
xiedeyantu:multi-line

Conversation

@xiedeyantu
Copy link
Copy Markdown
Contributor

No description provided.

@xiedeyantu
Copy link
Copy Markdown
Contributor Author

@julianhyde Could you help me to have a look this pr?

String[] parts = line.split(" ");
String propertyName = parts[1];
String valueString = parts[2];
while (valueString.startsWith("\"") && !valueString.endsWith("\"")) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

does this work for 3 lines?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add a test with multiple lines and I'll believe you. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have modified the test cases and readme.

@julianhyde
Copy link
Copy Markdown
Collaborator

Can you add tests for more than 2 lines, and tests where some of the lines are empty, and tests where line contains Quidem comment syntax.

Also update the documentation in README.md.

@xiedeyantu
Copy link
Copy Markdown
Contributor Author

@julianhyde I have modified the test cases and readme.Please have a look again thanks!

@julianhyde
Copy link
Copy Markdown
Collaborator

Thanks for the PR! I have merged (with a few revisions).

When you have successfully used these extensions in a Calcite PR (using 0.12-SNAPSHOT), let me know, and I will release Quidem version 0.12.

@xiedeyantu
Copy link
Copy Markdown
Contributor Author

Thanks for the PR! I have merged (with a few revisions).

When you have successfully used these extensions in a Calcite PR (using 0.12-SNAPSHOT), let me know, and I will release Quidem version 0.12.

OK, I will finish this task as soon as possible!

@xiedeyantu
Copy link
Copy Markdown
Contributor Author

@julianhyde I have tested it locally and it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants