Skip to content

feat: automatically detects IDE - windsurf or cursor - for AI context files. #7280

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 10, 2025

Conversation

smnh
Copy link
Contributor

@smnh smnh commented May 8, 2025

Summary

Instead of asking the user for which IDE they are using when running netlify recipes ai-context:
CleanShot 2025-05-08 at 22 48 50@2x

It now automatically detects from which IDE the command was run and adds the context files to the correctfolder:
CleanShot 2025-05-09 at 00 01 10@2x
CleanShot 2025-05-09 at 00 02 26@2x

If it didn't detect anything, it will continue to the original step where it presents a list to chose from.
User can also pass --skip-detection flag to skip the IDE detection logic:
CleanShot 2025-05-09 at 00 04 38@2x


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

@smnh smnh requested a review from a team as a code owner May 8, 2025 19:58
@smnh smnh requested review from sean-roberts and eduardoboucas and removed request for a team May 8, 2025 19:58
Copy link

github-actions bot commented May 8, 2025

📊 Benchmark results

Comparing with aaa87aa

  • Dependency count: 1,148 (no change)
  • Package size: 271 MB ⬆️ 0.00% increase vs. aaa87aa
  • Number of ts-expect-error directives: 390 (no change)

@smnh smnh force-pushed the smnh/detect-ide-for-ai-context branch from 1160a60 to e6214b2 Compare May 8, 2025 20:07
@smnh smnh force-pushed the smnh/detect-ide-for-ai-context branch from e6214b2 to 0fbfd50 Compare May 8, 2025 20:19
Added `--skip-detection` flag to skip the automatic IDE detection and fallback to choice list.
@@ -17,6 +17,10 @@ export const createRecipesCommand = (program: BaseCommand) => {
.argument('[name]', 'name of the recipe')
.description(`Create and modify files in a project using pre-defined recipes`)
.option('-n, --name <name>', 'recipe name to use')
.option(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will add a flag to the entire recipes command, not just this recipe. Right now the command itself is fully decoupled from the individual recipes, by design, and is not aware of any specific recipe. The fact that you're adding a parameter that is specific to a recipe, accompanied by help text that says it is relevant only to a specific recipe, breaks that.

If we keep using this pattern, we'll end up with a myriad of flags, each associated with a given recipe. I think that will get messy.

Do we really need this flag? Can we not just do the selection automatically when we detect thr IDE? If we really want an opt-out mechanism, I would recommend an environment variable as a stopgap solution until we build support for flags into the recipes themselves.

Copy link
Contributor Author

@smnh smnh May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, and will be happy to remove the --skip-detection flag.
@sean-roberts can we drop it? Asking because you ensisted on adding it 🫢
@eduardoboucas, is there a way right now to easily add this flag only to the ai-context receipe when user appends --help to the name of the recipe - netlify recipes ai-contexg --help?

]

const getPathByDetectingIDE = async (): Promise<string | null> => {
const getIDEFromCommand = (command: string): IDE | null => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Any reason for this function to be declared inside another function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, I can move it outside

return match ?? null
}

async function getCommandAndParentPID(pid: number): Promise<{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Any reason for this function to be declared inside another function? Also we have function declarations and function expressions in the same scope, might be good to standardise.

while (result.parentPID !== 1 && !result.ide) {
result = await getCommandAndParentPID(result.parentPID)
}
} catch (_) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
} catch (_) {
} catch {

@smnh
Copy link
Contributor Author

smnh commented May 9, 2025

@eduardoboucas removed --skip-detection flag and fixed other things you mentioned.

@smnh smnh requested a review from eduardoboucas May 9, 2025 11:11
// Start the download in the background while we wait for the prompts.
const download = downloadFile(version).catch(() => null)

const filePath = args[0] || (await promptForPath())
const filePath =
args[0] || ((options?.skipDetection ? null : await getPathByDetectingIDE()) ?? (await promptForPath()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can now remove references to options.skipDetection, right?

@@ -14,15 +14,16 @@ const SUGGESTION_TIMEOUT = 1e4
export interface RunRecipeOptions {
args: string[]
command?: BaseCommand
options?: OptionValues
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this anymore?

@sean-roberts sean-roberts enabled auto-merge (squash) May 9, 2025 20:38
@sean-roberts sean-roberts merged commit fd02e5d into main May 10, 2025
52 checks passed
@sean-roberts sean-roberts deleted the smnh/detect-ide-for-ai-context branch May 10, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants