-
Notifications
You must be signed in to change notification settings - Fork 2
Add a dda command to automatically sync cursor rules in other coding agent files #179
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
# SPDX-FileCopyrightText: 2025-present Datadog, Inc. <dev@datadoghq.com> | ||
# | ||
# SPDX-License-Identifier: MIT | ||
from __future__ import annotations | ||
|
||
from dda.cli.base import dynamic_group | ||
|
||
|
||
@dynamic_group( | ||
short_help="AI assistant tools and utilities", | ||
) | ||
def cmd() -> None: | ||
""" | ||
AI assistant tools and utilities for development workflow. | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
# SPDX-FileCopyrightText: 2025-present Datadog, Inc. <dev@datadoghq.com> | ||
# | ||
# SPDX-License-Identifier: MIT | ||
from __future__ import annotations | ||
|
||
from dda.cli.base import dynamic_group | ||
|
||
|
||
@dynamic_group( | ||
short_help="Manage AI rules for assistants", | ||
) | ||
def cmd() -> None: | ||
""" | ||
Manage AI rules for assistants such as Claude and Cursor. | ||
""" |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,154 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# SPDX-FileCopyrightText: 2025-present Datadog, Inc. <dev@datadoghq.com> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# SPDX-License-Identifier: MIT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from __future__ import annotations | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import TYPE_CHECKING | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import click | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from dda.cli.base import dynamic_command, pass_app | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from dda.utils.fs import Path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if TYPE_CHECKING: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from dda.cli.application import Application | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@dynamic_command(short_help="Sync cursor rules to other coding agent config file") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@click.option( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"--cursor-rules-path", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"-c", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"cursor_rules_path", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type=click.Path(exists=True, path_type=Path), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: should probably check that this is a directory and not a file
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
help="Path to the cursor rules directory", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
default=".cursor/rules", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@click.option( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"--sync-targets", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"-s", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"sync_targets", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type=str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
help="Comma separated list of targets files to sync the rules to. Defaults to Claude.MD file.", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
default="CLAUDE.md", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+27
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should avoid manually splitting along commas like this. Click has two methods for passing in an arbitrary number of arguments: Option 1:
|
@click.option( | |
"--sync-targets", | |
"-s", | |
"sync_targets", | |
type=str, | |
help="Comma separated list of targets files to sync the rules to. Defaults to Claude.MD file.", | |
default="CLAUDE.md", | |
) | |
@click.option( | |
"--sync-target", # Maybe we could simplify to --target as well ? | |
"-s", | |
"sync_targets", | |
type=click.Path(dir_okay=False, writable=True, path_type=Path), | |
multiple=True, | |
help="Target file to sync the rules to. Defaults to CLAUDE.md file. Can be passed multiple times", | |
default=[Path("CLAUDE.md")], | |
) |
Option 2: argument
with nargs=-1
I personally prefer this option, as it seems more inline with what we are trying to do.
Instead of using an option
we should use a click.argument
and pass nargs=-1
. This way we do not have to use -s
to specify files to sync and can pass in an arbitrary number of files on the CLI, like this:
dda ai rules sync CLAUDE.md AGENTS.md
The caveat is that we won't be able to set a default
cleanly like this, as setting a default
is not supported with nargs=-1
. We also lose the help
.
What we can do, however, is to add this to the body of our function:
if len(sync_targets) == 0:
sync_targets = (Path("CLAUDE.md"),)
Thus we would get:
@click.option( | |
"--sync-targets", | |
"-s", | |
"sync_targets", | |
type=str, | |
help="Comma separated list of targets files to sync the rules to. Defaults to Claude.MD file.", | |
default="CLAUDE.md", | |
) | |
@click.parameter( | |
"sync_targets", | |
type=click.Path(dir_okay=False, writable=True, path_type=Path), | |
nargs=-1 | |
) |
In either case this also changes the signature of the function below, see other comment
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.
Not necessary ?
cursor_rules_dir = Path(cursor_rules_path) |
Will also need to rename cursor_rules_dir
-> cursor_rules_path
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.
Required changes with above nargs=-1
change. Also a good idea to check for len(sync_targets) > 0
def cmd(app: Application, *, cursor_rules_path: Path, sync_targets: str, check_sync: bool) -> None: | |
cursor_rules_dir = Path(cursor_rules_path) | |
targets_files = [Path(target) for target in sync_targets.split(",")] | |
def cmd(app: Application, *, cursor_rules_path: Path, sync_targets: tuple[Path, ...], check_sync: bool) -> None: | |
cursor_rules_dir = Path(cursor_rules_path) | |
if len(sync_targets) == 0: | |
app.display_warning("Please specify at least one sync target") | |
return |
Will also need to rename targets_files
-> sync_targets
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.
if any(part == "personal" for part in root_path.parts): | |
if "personal" in root_path.parts: |
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.
You can simplify all of this by using glob
instead of manually walking the tree:
rule_files: list[Path] = [] | |
if cursor_rules_dir.is_dir(): | |
for root, _, files in os.walk(cursor_rules_dir): | |
root_path = Path(root) | |
# More robust check for personal rules using path parts | |
if any(part == "personal" for part in root_path.parts): | |
continue | |
rule_files.extend([root_path / file for file in files if file.endswith(".mdc")]) | |
# Sort files for consistent output | |
rule_files.sort() | |
return rule_files | |
return sorted(rule for rule in cursor_rules_dir.glob('**/*.mdc') if "personal" not in rule.parts) |
If you prefer keeping the more explicit tree-walk:
- we prefer to import modules inside functions, so as not to slow down the initialization of
dda
commands which happens at module import time (so moveimport os
in the body of the function). This does mean the module would get imported once for each function run, but since only one command is run beforedda
exits, this function will only be called once anyway. - since we use Python >= 3.12, you can use
Path.walk
doc instead ofos.walk
, which avoid needing to import os and needing to convertstr
object toPath
s on each loop
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.
Presumably this should not be specific to Claude ? Do you think Claude will have issues understanding it if we don't refer to it by name but instead with a generic "AI assistants" ? (I guess we could say "AI Assistants such as Claude")
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.
nit: clearer imo
- globs: array of strings, the file globs to apply the rule to | |
- globs: array of strings, glob patterns specifying which files to apply the rule to |
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.
nit: Not sure why you have a check for content
here, shouldn't it always be non-empty anyway ?
Also I'd argue that even if content
is empty, if we pass in a target on the CLI we should expect it to exist, even if it is supposed to be empty (not sure when that would happen but...)
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.
I'd also add a debug output in this branch, no reason why we wouldn't have one here since we have one everywhere else
unsynced_targets.append(str(target_file)) | |
unsynced_targets.append(str(target_file)) | |
app.display_debug(f"Error: {target_file} does not exist !") |
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.
I'd probably reduce verbosity here, this can be kinda noisy I think. Maybe we can have a single info-level log at the beginning showing the entire list of things to check.
app.display_info(f"Checking if {target_file} is in sync with {cursor_rules_dir}") | |
app.display_debug(f"Checking if {target_file} is in sync with {cursor_rules_dir}") |
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.
Same idea here, this is pretty noisy if we also have the "summary one" at the end. However I couldn't find a way to decouple the log level and the display style using the dda.display_*
functions, so we'd have to use the standard debug style:
app.display_error(f"Error: {target_file} is not in sync with {cursor_rules_dir}") | |
app.display_debug(f"Error: {target_file} is not in sync with {cursor_rules_dir}") |
I'll bring this issue up with Ofek, we should probably have a way to override either style or log level in the display
functions, in case we want to, like here, display debug
things with an error style for example
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.
nit: Use continue
unsynced_targets.append(str(target_file)) | |
else: | |
app.display_info(f"Checking if {target_file} is in sync with {cursor_rules_dir}") | |
target_content = target_file.read_text(encoding="utf-8") | |
if target_content != content: | |
app.display_error(f"Error: {target_file} is not in sync with {cursor_rules_dir}") | |
unsynced_targets.append(str(target_file)) | |
unsynced_targets.append(str(target_file)) | |
continue | |
app.display_info(f"Checking if {target_file} is in sync with {cursor_rules_dir}") | |
target_content = target_file.read_text(encoding="utf-8") | |
if target_content != content: | |
app.display_error(f"Error: {target_file} is not in sync with {cursor_rules_dir}") | |
unsynced_targets.append(str(target_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.
app.display_info(f"Syncing {len(rule_files)} rule files to {target_file}") | |
app.display_debug(f"Syncing {len(rule_files)} rule files to {target_file}") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# SPDX-FileCopyrightText: 2025-present Datadog, Inc. <dev@datadoghq.com> | ||
# | ||
# SPDX-License-Identifier: MIT |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# SPDX-FileCopyrightText: 2025-present Datadog, Inc. <dev@datadoghq.com> | ||
# | ||
# SPDX-License-Identifier: MIT |
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.
nit: IIRC we prefer to use verbal phrases for help texts, so maybe something like
would be better ?