- Notifications
You must be signed in to change notification settings - Fork914
fix: locate Terraform entrypoint file#16753
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| if (filename === defaultMainTerraformFile) { | ||
| initialFile = path; | ||
| skip = true; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What is the order in whichtraverse() will traverse the nodes offileTree? Are child paths sorted prior to being added to the frontier? In other words, if you have botha/main.tf andz/main.tf, which will get chosen? I know you have a test above, but is the output dependant on the ordering of the nodes? Is it just "first found wins"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Let me expandtraverse() with sorting capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nice 👍
930816f intomainUh oh!
There was an error while loading.Please reload this page.
| @@ -90,7 +90,7 @@ export const TemplateVersionEditorPage: FC = () => { | |||
| // File navigation | |||
| // It can be undefined when a selected file is deleted | |||
| const activePath: string | undefined = | |||
| searchParams.get("path") ??findInitialFile(fileTree ?? {}); | |||
| searchParams.get("path") ??findEntrypointFile(fileTree ?? {}); | |||
ParkreinerFeb 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do you know what happens ifsearchParams.get returns an empty string or a string that isn't a valid filename?
That would short-circuit the nullish expression and preventfindEntrypointFile from ever being called, and as far as I can tell,activePath gets fed directly into the editor without any additional data massaging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nice catch! let me look into this 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| // 1. If "main.tf" exists in root, return it. | ||
| // 2. Traverse through sub-directories. | ||
| // 3. If "main.tf" exists in a sub-directory, skip further browsing, and return the path. | ||
| // 4. If "main.tf" was not found, return the last reviewed "".tf" file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
With the sorting in place now, I feel like it would've helped to clarify what "last reviewed" means, especially to help make it clear that the tests are actually deterministic
Fixes:#16360
This PR updates the template version editor code to accurately identify the Terraform entrypoint file.