Added escaping capability within quoted parametters for input. #142
+72
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi, a while has passed and nothing has been done. Is this commit not good enough? Is there something that blocks to accept the code? Maybe i could help somehow, if possible. It is just that when the code is left aside for so long things are starting to be forgotten. There were some more things that i wanted to implement, but before that wanted so the current situation is resolved, so the merging is easier. |
|
I haven't made time myself to review this. I welcome others to review the
code and try the patch themselves.
This is a medium-sized patch, to me, and it would be easier to evaluate
quickly if there were tests added to cover the new features.
Could you add some tests for this? It would help ensure also that future
changes don't break this code.
…On Tue, Nov 22, 2016 at 12:25 AM abensj ***@***.***> wrote:
Hi, a while has passed and nothing has been done.
Is this commit not good enough? Is there something that blocks to accept
the code? Maybe i could help somehow, if possible.
It is just that when the code is left aside for so long things are
starting to be forgotten. There were some more things that i wanted to
implement, but before that wanted so the current situation is resolved, so
the merging is easier.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#142 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAIC6ssOR6gt1nUvIkTqpHIaGXIwSzinks5rAqcEgaJpZM4JvB5Q>
.
|
|
Ok, I'll take a look at how to write/extend some tests. It will take a while though. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When input was read from pipe or file, quoted parameters could not contain quotes themselves and there was no way to escape them. For example if command 'search' should find a window that is named <Some text "that's good" >, it was not possible to do unless changing some of quotes with regex dot to match them.
With this upgrade it is possible to escape quotes with backslash just like in most commonly used programming languages, for example, search "Some text "that's good"" or search 'Some text "that's good"'.
Now escaping works with both types of quotes. Philosophical question though remains whether escapes should work in both types of quotes. There are languages which allow only to escape single quote and backslash only when using single quotes and extended escapes when using double quotes. At the moment since extended escaping is not implemented it maybe does not matter that much and for simplicity same functionality was implemented for both quote types. But maybe if we decide that now, could add comment in code for future.
Theoretically this upgrade should not influence previous functionality anyhow, since quote closing is handled almost the same way as previously.
It was tested only with search command, but it could be possible that other commands could benefit from this upgrade too. It was not tested with newline, tab and similar regex special escapes, but it should work as well, because those are just passed through unchanged.