nginxinc / crossplane

Quick and reliable way to convert NGINX configurations into JSON and back.
Apache License 2.0
717 stars 86 forks source link

Bugfix: Wrong filenames after includes in certain scenarios #95

Closed pbitty closed 1 year ago

pbitty commented 3 years ago

Proposed changes

This fixes a bug that occurs 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 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

pbitty commented 3 years ago

Hello. Is anyone available to give some feedback on this? If I am not following the right process, please let me know.

pbitty commented 2 years ago

Ping! :)

Is anyone around to give some feedback on this PR?

defanator commented 2 years ago

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.

pbitty commented 2 years ago

Hi @defanator, no problem at all, I appreciate your response.