Quote stripping in log command #4429
Closed
Conversation
This file contains hidden or 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
KrystalDelusion
changed the title
widlarizer
reviewed
Jul 15, 2024
|
|
||
| test_log "test" "test" | ||
| test_log "test;" "test" | ||
| test_log "test;;" "test" |
There was a problem hiding this comment.
Allows e.g.
log "yosys> flatten;;", logging the unquoted textyosys> flatten;;.
Doesn't this line test that the semicolons would be discarded?
There was a problem hiding this comment.
Yes, contrast with the later test_log "\"test;;\"" "test;;" which tests if putting them in quotes prevents removing them, i.e. "\"test;;\"" -> log "test;;" because bash doesn't include the outer quotes when doing the variable substitution.
widlarizer
approved these changes
Jul 17, 2024
KrystalDelusion
added
the
merge-after-jf
Currently provides tests for logging quoted strings.
We can now fake the `yosys> flatten;;` line, so we no longer need to explain the split flatten/clean.
If a command has a single argument, and that argument is quoted, strip the quotes.
`log "` should log `"`. Also fix test script to correctly fail when more than one test fails.
KrystalDelusion
force-pushed
the
krys/quotes
branch
from
a26f648 to
7d3c69e
Compare
KrystalDelusion
removed
the
merge-after-jf
KrystalDelusion
added
the
status-paused
|
@KrystalDelusion should I close / move to backlog? |
widlarizer
added
status-superseded
|
Superseded by #5385 |
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.
What are the reasons/motivation for this change?
Allows e.g.
log "yosys> flatten;;", logging the unquoted textyosys> flatten;;.Explain how this is achieved.
If the text to log starts and ends with a quotation mark, drop the first and last characters.
If applicable, please suggest to reviewers how they can test the change.