|
| 1 | +# GitHub Copilot Instructions for ast-merge |
| 2 | + |
| 3 | +This document contains important information for AI assistants working on this codebase. |
| 4 | + |
| 5 | +## Tool Usage Preferences |
| 6 | + |
| 7 | +### Prefer Internal Tools Over Terminal Commands |
| 8 | + |
| 9 | +**IMPORTANT**: Copilot cannot see terminal output. Every terminal command requires the user to manually copy/paste the output back to the chat. This is slow and frustrating. |
| 10 | + |
| 11 | +✅ **PREFERRED** - Use internal tools: |
| 12 | +- `grep_search` instead of `grep` command |
| 13 | +- `file_search` instead of `find` command |
| 14 | +- `read_file` instead of `cat` command |
| 15 | +- `list_dir` instead of `ls` command |
| 16 | +- `replace_string_in_file` or `create_file` instead of `sed` / manual editing |
| 17 | + |
| 18 | +❌ **AVOID** when possible: |
| 19 | +- `run_in_terminal` for information gathering |
| 20 | +- Running grep/find/cat in terminal |
| 21 | + |
| 22 | +Only use terminal for: |
| 23 | +- Running tests (`bundle exec rspec`) |
| 24 | +- Installing dependencies (`bundle install`) |
| 25 | +- Git operations that require interaction |
| 26 | +- Commands that actually need to execute (not just gather info) |
| 27 | + |
| 28 | +## Search Tool Limitations |
| 29 | + |
| 30 | +### grep_search and Nested Git Projects |
| 31 | + |
| 32 | +**CRITICAL**: The `vendor/*` directories in this workspace are **nested git projects** (they have their own `.git/` directory, separated from the parent by gitignore patterns). The `grep_search` tool **CANNOT search inside nested git projects** - it only searches the main workspace. |
| 33 | + |
| 34 | +To search inside vendor gems: |
| 35 | +1. Use `read_file` to read specific files directly (this always works) |
| 36 | +2. Use `list_dir` to explore the directory structure |
| 37 | +3. Do NOT rely on `grep_search` with `includePattern: "vendor/**"` - it will return no results |
| 38 | + |
| 39 | +**Working approach for finding code in vendor gems:** |
| 40 | +``` |
| 41 | +# Step 1: List the directory structure |
| 42 | +list_dir("/home/pboling/src/kettle-rb/ast-merge/vendor/markdown-merge/lib/markdown/merge") |
| 43 | +
|
| 44 | +# Step 2: Read specific files you need to search |
| 45 | +read_file("/home/pboling/src/kettle-rb/ast-merge/vendor/markdown-merge/lib/markdown/merge/file_analysis.rb", 0, 100) |
| 46 | +
|
| 47 | +# Step 3: If looking for a specific method, read more of the file or multiple files |
| 48 | +``` |
| 49 | + |
| 50 | +### grep_search includePattern (for non-nested-git directories) |
| 51 | + |
| 52 | +**IMPORTANT**: The `includePattern` parameter uses glob patterns relative to the workspace root. |
| 53 | + |
| 54 | +✅ **WORKS** - Use these patterns: |
| 55 | +``` |
| 56 | +# Search recursively within a directory (use ** for recursive) |
| 57 | +includePattern: "vendor/**" # All files under vendor/ |
| 58 | +includePattern: "vendor/kettle-dev/**" # All files under vendor/kettle-dev/ |
| 59 | +
|
| 60 | +# Search a specific file |
| 61 | +includePattern: "vendor/prism-merge/README.md" |
| 62 | +includePattern: "lib/ast/merge/freezable.rb" |
| 63 | +
|
| 64 | +# Search files matching a pattern in a specific directory |
| 65 | +includePattern: "spec/**" # All spec files recursively |
| 66 | +``` |
| 67 | + |
| 68 | +❌ **DOES NOT WORK** - Avoid these: |
| 69 | +``` |
| 70 | +# The | character does NOT work for alternation in includePattern |
| 71 | +includePattern: "vendor/prism-merge/**|vendor/kettle-dev/**" |
| 72 | +
|
| 73 | +# Cannot use ** in the middle of a path with file extension |
| 74 | +includePattern: "vendor/**/spec/**/*.rb" # Too complex, may fail |
| 75 | +``` |
| 76 | + |
| 77 | +**Key insights:** |
| 78 | +- `vendor/**` searches ALL files recursively under vendor/ (including subfolders) |
| 79 | +- Without `includePattern`, `grep_search` searches the ENTIRE workspace (root project only, may miss vendor submodules) |
| 80 | +- For vendor gems, always use `includePattern: "vendor/**"` or `includePattern: "vendor/gem-name/**"` |
| 81 | +- To search multiple specific locations, make separate `grep_search` calls |
| 82 | + |
| 83 | +### replace_string_in_file and Unicode Characters |
| 84 | + |
| 85 | +**IMPORTANT**: The `replace_string_in_file` tool can fail silently when files contain special Unicode characters like: |
| 86 | +- Curly apostrophes (`'` U+2019 instead of `'`) |
| 87 | +- Em-dashes (`—` U+2014 instead of `-`) |
| 88 | +- Emoji (🔧, 🎨, etc.) |
| 89 | + |
| 90 | +When `replace_string_in_file` fails with "Could not find matching text": |
| 91 | +1. The file likely contains Unicode characters that don't match what you're sending |
| 92 | +2. Try using a smaller, more unique portion of the text |
| 93 | +3. Avoid including lines with emojis or special punctuation in your oldString |
| 94 | +4. Use `read_file` to see the exact content, but be aware the display may normalize characters |
| 95 | + |
| 96 | +## Project Structure |
| 97 | + |
| 98 | +- `lib/ast/merge/` - Base library classes (ast-merge gem) |
| 99 | +- `vendor/prism-merge/` - Ruby/Prism-specific merge implementation |
| 100 | +- `vendor/*/` - Other format-specific merge implementations (markly-merge, json-merge, etc.) |
| 101 | + |
| 102 | +## API Conventions |
| 103 | + |
| 104 | +### SmartMergerBase API |
| 105 | +- `merge` - Returns a **String** (the merged content) |
| 106 | +- `merge_result` - Returns a **MergeResult** object |
| 107 | +- `to_s` on MergeResult returns the merged content as a string |
| 108 | + |
| 109 | +### Comment Classes |
| 110 | +- `Ast::Merge::Comment::*` - Generic, language-agnostic comment classes |
| 111 | +- `Prism::Merge::Comment::*` - Ruby-specific comment classes with magic comment detection |
| 112 | + |
| 113 | +### Naming Conventions |
| 114 | +- File paths must match class paths (Ruby convention) |
| 115 | +- Example: `Ast::Merge::Comment::Line` → `lib/ast/merge/comment/line.rb` |
| 116 | + |
| 117 | +## Architecture Notes |
| 118 | + |
| 119 | +### prism-merge (as of December 2024) |
| 120 | +- Uses section-based merging with recursive body merging |
| 121 | +- Does NOT use FileAligner or ConflictResolver (these were removed as vestigial) |
| 122 | +- SmartMerger handles all merge logic directly |
| 123 | +- Comment-only files are handled via `Ast::Merge::Text::SmartMerger` |
| 124 | + |
| 125 | +## Loading Vendor Gems in Scripts |
| 126 | + |
| 127 | +**IMPORTANT**: When writing standalone Ruby scripts to test vendor gems, you must use `bundler/setup` to properly load the gems. |
| 128 | + |
| 129 | +✅ **CORRECT** - Use bundler/setup: |
| 130 | +```ruby |
| 131 | +#!/usr/bin/env ruby |
| 132 | +# frozen_string_literal: true |
| 133 | + |
| 134 | +require "bundler/setup" |
| 135 | +require "prism/merge" |
| 136 | + |
| 137 | +# Now you can use Prism::Merge classes |
| 138 | +``` |
| 139 | + |
| 140 | +❌ **BROKEN** - These do NOT work: |
| 141 | +```ruby |
| 142 | +# This doesn't load the gem properly: |
| 143 | +require_relative "lib/prism/merge" |
| 144 | + |
| 145 | +# This doesn't set up the load path: |
| 146 | +require "prism/merge" # without bundler/setup first |
| 147 | +``` |
| 148 | + |
| 149 | +The pattern `require "bundler/setup"` followed by `require "gem/name"` works because: |
| 150 | +1. `bundler/setup` configures the load path based on the Gemfile |
| 151 | +2. The vendor gems are specified in the Gemfile with `path:` option |
| 152 | +3. This allows standard `require` to find the gems |
| 153 | + |
| 154 | +## Testing |
| 155 | + |
| 156 | +Run tests from the appropriate directory: |
| 157 | +```bash |
| 158 | +# ast-merge tests |
| 159 | +cd /var/home/pboling/src/kettle-rb/ast-merge |
| 160 | +bundle exec rspec spec/ |
| 161 | + |
| 162 | +# prism-merge tests |
| 163 | +cd /var/home/pboling/src/kettle-rb/ast-merge/vendor/prism-merge |
| 164 | +bundle exec rspec spec/ |
| 165 | +``` |
| 166 | + |
| 167 | +### Coverage Reports |
| 168 | + |
| 169 | +To generate a coverage report for any vendor gem: |
| 170 | +```bash |
| 171 | +cd /var/home/pboling/src/kettle-rb/ast-merge/vendor/prism-merge # or other vendor gem |
| 172 | +bin/rake coverage && bin/kettle-soup-cover -d |
| 173 | +``` |
| 174 | + |
| 175 | +This runs tests with coverage instrumentation and generates detailed coverage reports in the `coverage/` directory. |
| 176 | + |
| 177 | +## Common Pitfalls |
| 178 | + |
| 179 | +1. **NEVER add backward compatibility** - The maintainer explicitly prohibits backward compatibility shims, aliases, or deprecation layers. Make clean breaks. |
| 180 | +2. **Magic comments** - Ruby-specific, belong in prism-merge not ast-merge |
| 181 | +3. **`content_string` is legacy** - Use `to_s` instead |
| 182 | +4. **`merged_source` doesn't exist** - `merge` returns a String directly |
| 183 | + |
| 184 | +## Terminal Command Restrictions |
| 185 | + |
| 186 | +### Terminal Session Management |
| 187 | + |
| 188 | +**How `run_in_terminal` works**: |
| 189 | +- The tool sends commands to a **single persistent Copilot terminal** |
| 190 | +- Commands run in sequence in the same terminal session |
| 191 | +- Environment variables and working directory persist between calls |
| 192 | +- The first command in a session either does not run at all, or runs before the shell initialization (direnv, motd, etc.) so it should always be a noop, like `true`. |
| 193 | + |
| 194 | +**When things go wrong**: |
| 195 | +- If output shows only shell banner/motd without command results, the command most likely worked, but the tool has lost the ability to see terminal output. This happens FREQUENTLY. |
| 196 | +- EVERY TIME you do not see output, STOP and confirm output status with the user. |
| 197 | +- Solution: Ask the user to share the output they see. |
| 198 | + |
| 199 | +**Best practices**: |
| 200 | +1. **Combine related commands** - Use `&&` to chain commands: |
| 201 | + ```bash |
| 202 | + cd /path && bundle exec rspec spec/some_spec.rb |
| 203 | + ``` |
| 204 | +2. **Use `get_terminal_output`** - Check output of background processes |
| 205 | +3. **Prefer internal tools** - Use `grep_search`, `read_file`, `list_dir` instead of terminal for information gathering |
| 206 | + |
| 207 | +### NEVER Pipe Test Commands Through head/tail |
| 208 | + |
| 209 | +**CRITICAL**: NEVER use `head`, `tail`, or any output truncation with test commands (`rspec`, `rake`, `bundle exec rspec`, etc.). |
| 210 | + |
| 211 | +❌ **ABSOLUTELY FORBIDDEN**: |
| 212 | +```bash |
| 213 | +bundle exec rspec 2>&1 | tail -50 |
| 214 | +bin/rake coverage 2>&1 | head -100 |
| 215 | +bin/rspec | tail -200 |
| 216 | +``` |
| 217 | + |
| 218 | +✅ **CORRECT** - Run commands without truncation: |
| 219 | +```bash |
| 220 | +bundle exec rspec |
| 221 | +bin/rake coverage |
| 222 | +bin/rspec |
| 223 | +``` |
| 224 | + |
| 225 | +**Why**: |
| 226 | +- You cannot predict how much output a test run will produce |
| 227 | +- Your predictions are ALWAYS wrong |
| 228 | +- You cannot see terminal output anyway - the user will copy relevant portions for you |
| 229 | +- Truncating output often hides the actual errors or relevant information |
| 230 | +- The user knows what's important and will share it with you |
0 commit comments