-
Notifications
You must be signed in to change notification settings - Fork 28
feat(create): update app name in 'pyproject.toml' and 'package.json' #344
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 |
|---|---|---|
|
|
@@ -48,16 +48,26 @@ func NewClient( | |
|
|
||
| // UpdateDefaultProjectFiles should update any project specific files if any | ||
| func UpdateDefaultProjectFiles(fs afero.Fs, dirPath string, appDirName string) error { | ||
| var filenames = []string{"manifest.json", "manifest.js", "manifest.ts"} | ||
| // Files and their corresponding app name replacement functions | ||
| projectFiles := []struct { | ||
| filename string | ||
| replacer func([]byte, string) []byte | ||
| }{ | ||
| {"manifest.json", regexReplaceAppNameInManifest}, | ||
| {"manifest.js", regexReplaceAppNameInManifest}, | ||
| {"manifest.ts", regexReplaceAppNameInManifest}, | ||
| {"package.json", regexReplaceAppNameInPackageJSON}, | ||
| {"pyproject.toml", regexReplaceAppNameInPyprojectToml}, | ||
| } | ||
|
|
||
| for _, filename := range filenames { | ||
| filePath := filepath.Join(dirPath, filename) | ||
| for _, pf := range projectFiles { | ||
| filePath := filepath.Join(dirPath, pf.filename) | ||
| fileData, err := afero.ReadFile(fs, filePath) | ||
| if err != nil { | ||
| continue | ||
| } | ||
|
|
||
| fileData = regexReplaceAppNameInManifest(fileData, appDirName) | ||
| fileData = pf.replacer(fileData, appDirName) | ||
| if err := afero.WriteFile(fs, filePath, fileData, 0644); err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -149,3 +159,17 @@ func regexReplaceAppNameInManifest(src []byte, appName string) []byte { | |
|
|
||
| return srcUpdated | ||
| } | ||
|
|
||
| // regexReplaceAppNameInPackageJSON replaces the top-level "name" field in a package.json file | ||
| func regexReplaceAppNameInPackageJSON(src []byte, appName string) []byte { | ||
| re := regexp.MustCompile(`(?m)^(\s*"name"\s*:\s*")([^"]*)(")`) | ||
| repl := fmt.Sprintf("${1}%s${3}", appName) | ||
| return re.ReplaceAll(src, []byte(repl)) | ||
| } | ||
|
|
||
| // regexReplaceAppNameInPyprojectToml replaces the "name" field under the [project] section in a pyproject.toml file | ||
| func regexReplaceAppNameInPyprojectToml(src []byte, appName string) []byte { | ||
| re := regexp.MustCompile(`(\[project\][^\[]*?name\s*=\s*")([^"]*)(")`) | ||
| repl := fmt.Sprintf("${1}%s${3}", appName) | ||
| return re.ReplaceAll(src, []byte(repl)) | ||
| } | ||
|
Comment on lines
+170
to
+175
Member
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. note: I've confirmed that this will only match While we're using teh |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,40 @@ func Test_App_UpdateDefaultProjectFiles(t *testing.T) { | |
| }, | ||
| expectedErrorType: nil, | ||
| }, | ||
| "package.json file exists": { | ||
| appDirName: "vibrant-butterfly-1234", | ||
| existingFiles: map[string]string{ | ||
| "package.json": string(testdata.PackageJSON), | ||
| }, | ||
| expectedFiles: map[string]string{ | ||
| "package.json": string(testdata.PackageJSONAppName), | ||
| }, | ||
| expectedErrorType: nil, | ||
| }, | ||
| "pyproject.toml file exists": { | ||
| appDirName: "vibrant-butterfly-1234", | ||
| existingFiles: map[string]string{ | ||
| "pyproject.toml": string(testdata.PyprojectTOML), | ||
| }, | ||
| expectedFiles: map[string]string{ | ||
| "pyproject.toml": string(testdata.PyprojectTOMLAppName), | ||
| }, | ||
| expectedErrorType: nil, | ||
| }, | ||
| "Multiple project files exist": { | ||
| appDirName: "vibrant-butterfly-1234", | ||
| existingFiles: map[string]string{ | ||
| "manifest.json": string(testdata.ManifestJSON), | ||
| "package.json": string(testdata.PackageJSON), | ||
| "pyproject.toml": string(testdata.PyprojectTOML), | ||
| }, | ||
| expectedFiles: map[string]string{ | ||
| "manifest.json": string(testdata.ManifestJSONAppName), | ||
| "package.json": string(testdata.PackageJSONAppName), | ||
| "pyproject.toml": string(testdata.PyprojectTOMLAppName), | ||
| }, | ||
| expectedErrorType: nil, | ||
| }, | ||
| "No manifest files exist": { | ||
| appDirName: "vibrant-butterfly-1234", | ||
| existingFiles: map[string]string{}, | ||
|
|
@@ -161,3 +195,43 @@ func Test_RegexReplaceAppNameInManifest(t *testing.T) { | |
| }) | ||
| } | ||
| } | ||
|
|
||
| func Test_RegexReplaceAppNameInPackageJSON(t *testing.T) { | ||
|
Member
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. suggestion: Can we add a test where We could put the {
"name": "vibrant-butterfly-1234",
"version": "1.0.0",
"description": "A Slack app built with Bolt",
"main": "app.js",
"scripts": {
"start": "node app.js"
},
"dependencies": {
"@slack/bolt": "^4.0.0"
},
"config": {
"name": "local-server-name",
"host": "localhost",
"port": "8080"
}
} |
||
| tests := map[string]struct { | ||
| src []byte | ||
| appName string | ||
| expectedSrc []byte | ||
| }{ | ||
| "package.json name is replaced": { | ||
| src: testdata.PackageJSON, | ||
| appName: "vibrant-butterfly-1234", | ||
| expectedSrc: testdata.PackageJSONAppName, | ||
| }, | ||
| } | ||
| for name, tc := range tests { | ||
| t.Run(name, func(t *testing.T) { | ||
| actualSrc := regexReplaceAppNameInPackageJSON(tc.src, tc.appName) | ||
| require.Equal(t, tc.expectedSrc, actualSrc) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func Test_RegexReplaceAppNameInPyprojectToml(t *testing.T) { | ||
|
Member
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. suggestion: Likewise, I think we should add an extra An example could be: |
||
| tests := map[string]struct { | ||
| src []byte | ||
| appName string | ||
| expectedSrc []byte | ||
| }{ | ||
| "pyproject.toml name is replaced": { | ||
| src: testdata.PyprojectTOML, | ||
| appName: "vibrant-butterfly-1234", | ||
| expectedSrc: testdata.PyprojectTOMLAppName, | ||
| }, | ||
| } | ||
| for name, tc := range tests { | ||
| t.Run(name, func(t *testing.T) { | ||
| actualSrc := regexReplaceAppNameInPyprojectToml(tc.src, tc.appName) | ||
| require.Equal(t, tc.expectedSrc, actualSrc) | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| { | ||
| "name": "vibrant-butterfly-1234", | ||
| "version": "1.0.0", | ||
| "description": "A Slack app built with Bolt", | ||
| "main": "app.js", | ||
| "scripts": { | ||
| "start": "node app.js" | ||
| }, | ||
| "dependencies": { | ||
| "@slack/bolt": "^4.0.0" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| { | ||
| "name": "bolt-app-template", | ||
| "version": "1.0.0", | ||
| "description": "A Slack app built with Bolt", | ||
| "main": "app.js", | ||
| "scripts": { | ||
| "start": "node app.js" | ||
| }, | ||
| "dependencies": { | ||
| "@slack/bolt": "^4.0.0" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,16 @@ | ||||||||
| [project] | ||||||||
| name = "vibrant-butterfly-1234" | ||||||||
| version = "0.1.0" | ||||||||
| requires-python = ">=3.9" | ||||||||
| dependencies = [ | ||||||||
| "slack-sdk==3.40.0", | ||||||||
| "slack-bolt==1.27.0", | ||||||||
| "slack-cli-hooks<1.0.0", | ||||||||
| ] | ||||||||
|
|
||||||||
| [tool.ruff] | ||||||||
|
Member
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.
Suggested change
👾 suggestion(non-blocking): I understand the advantage of using regex to be comments and indentation kept during replacements. Using a package like
Member
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. Unfortunately, I don't think we should do that. I've experimented with |
||||||||
| [tool.ruff.lint] | ||||||||
| [tool.ruff.format] | ||||||||
|
|
||||||||
| [tool.pytest.ini_options] | ||||||||
| testpaths = ["tests"] | ||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,16 @@ | ||||||||
| [project] | ||||||||
| name = "bolt-python-ai-agent-template" | ||||||||
| version = "0.1.0" | ||||||||
| requires-python = ">=3.9" | ||||||||
| dependencies = [ | ||||||||
| "slack-sdk==3.40.0", | ||||||||
| "slack-bolt==1.27.0", | ||||||||
| "slack-cli-hooks<1.0.0", | ||||||||
| ] | ||||||||
|
|
||||||||
| [tool.ruff] | ||||||||
|
Member
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.
Suggested change
🔭 suggestion(non-blocking): This comment follows the one before! |
||||||||
| [tool.ruff.lint] | ||||||||
| [tool.ruff.format] | ||||||||
|
|
||||||||
| [tool.pytest.ini_options] | ||||||||
| testpaths = ["tests"] | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,3 +27,15 @@ var ManifestSDKTS []byte | |
|
|
||
| //go:embed manifest-sdk-app-name.ts | ||
| var ManifestSDKTSAppName []byte | ||
|
|
||
| //go:embed package.json | ||
| var PackageJSON []byte | ||
|
|
||
| //go:embed package-app-name.json | ||
| var PackageJSONAppName []byte | ||
|
|
||
| //go:embed pyproject.toml | ||
| var PyprojectTOML []byte | ||
|
|
||
| //go:embed pyproject-app-name.toml | ||
| var PyprojectTOMLAppName []byte | ||
|
Comment on lines
+31
to
+41
Member
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. praise: Great use of the embeds to keep test data clean and easy to maintain! |
||
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.
suggestion(blocking): I think this regex will match all
"name"keys inpackage.jsoninstead of just the top-level "name" field. While other"name"keys are rare, I think that's a little too risky.A few ideas:
$2is the name field:(.*\{[^{]*"name"\s*:\s*")([^"]*)(".*)npm init(?m)^(\s{2}"name"\s*:\s*")([^"]*)(")"name"key exists (better to error on doing nothing than corrupting thepackage.jsonfile)Here is an example of multiple matches and why we'd need a fix: