Tracking PR for: "Cygwin: rewrite cmdline parser" #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
This commit rewrites the cmdline parser to achieve the following: * MSVCRT compatibility. Except for the single-quote handling (an extension for compatibility with old Cygwin), the parser now interprets option boundaries exactly like MSVCR since 2008. This fixes the issue where our escaping does not work with our own parsing. * Clarity. Since globify() is no longer responsible for handling the opening and closing of quotes, the code is much simpler. * Sanity. The GLOB_NOCHECK flag is removed, so a failed glob correctly returns the literal value. Without the change, anything path-like would be garbled by globify's escaping. * A memory leak in the @file expansion is removed by rewriting it to use a stack of buffers. This also simplifies the code since we no longer have to move stuff. The "downside" is that tokens can no longer cross file boundaries. Some clarifications are made in the documentation for when globs are not expanded. The change fixes two complaints of mine: * That cygwin is incompatible with its own escape.[1] * That there is no way to echo `C:\"` from win32.[2] [1]: https://cygwin.com/pipermail/cygwin/2020-June/245162.html [2]: https://cygwin.com/pipermail/cygwin/2019-October/242790.html (It's never the point to spawn cygwin32 from cygwin64. Consistency matters: with yourself always, and with the outside world when you are supposed to.) This is the fifth version of the patch, applying reviewer suggestions to keep the interface private. I provide all my patches to Cygwin, including this one and all future ones, under the 2-clause BSD license.
This was referenced Aug 28, 2023
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.
This is a patch I wrote two years ago and never had time to finalize, but the issue still pops up and bites me from time to time. I figured that even though GitHub still isn't the place to submit patches, the CI and merge status indications are useful. So there, a PR to help me clean stuff up when I prepare for PATCHv7 someday.
The last submission seems to be https://cygwin.com/pipermail/cygwin-patches/2021q2/011367.html. Apparently it's newer than what we got here, since this is v5 and that is v6. Oops. Guess I need to get to that first.