Bugfix: Wrong filenames after includes in certain scenarios #95
Conversation
This bug manifests itself when an include exists in the middle of a file and `Combine=True` is set. The directives that appear after the include in the outer file will have the wrong filename on them, subject to the number of directives in the included file.
The include-handling block in `_parse` had the loop variable `fname` which was mutating the value of the outer `fname` variable from line 74. By moving this to its own method, we make more clear which variables are in the method's scope vs an outer scope.
pbitty
changed the title
|
Hello. Is anyone available to give some feedback on this? If I am not following the right process, please let me know. |
|
Ping! :) Is anyone around to give some feedback on this PR? |
|
Hi @pbitty - thanks for your PR. Indeed, this is a correct place to post any fixes around crossplane, so you're doing it right - no worries at all! Unfortunately, the project is a bit stalled nowadays - but we still use it as a part of NGINX Amplify, so any contributions, including bug fixes, are greatly valued. I hope to see some improvements on regular process of checking this and other Amplify-related public repositories for reports and issues; in the meantime, let me try to seek for appropriate resources to review your particular one. Sorry for not being as responsive as we could, and thank you again for your attention. |
|
Hi @defanator, no problem at all, I appreciate your response. |
Proposed changes
This fixes a bug that occurs when an include exists in the middle of a file and
Combine=Trueis set. The directives that appear after the include in the outer file will have the wrong filename on them, subject to the number of directives in the included file.The failing test in the first commit exposes the bug. This bug was caused by a loop variable mutating the value of a variable by the same name in an outer scope.
Checklist