feat!: Support for XML/HTML tags on the API calls and a myriad of oth… #25
Conversation
…er things (#20) * 📝 Add badge to README.md * 📝 change usage text * 💪 Delete `stdin` flag * 📝 Update README * ⤴ Update version of library * 🥷 divide package * 🐛 fix test and ci * 💪 Add test * 🐛 Fix default config directory permission to 0755 To avoid "permission denied" * Add renovate.json (#11) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore: use conventional commit (#15) * Update module github.com/mattn/go-isatty to v0.0.19 (#12) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * fix(deps): update module github.com/urfave/cli/v2 to v2.25.7 (#13) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update actions/checkout action to v4 (#16) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update actions/setup-go action to v4 (#17) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update goreleaser/goreleaser-action action to v5 (#18) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * build: update golang version * ci: update ci * ci: fix go-version * fix(deps): update module github.com/mattn/go-isatty to v0.0.20 (#19) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore: update .goreleaser * build: update go.sum * Chore: small fixes Mostly English spelling and getting rid of deprecated Go functions. * chore: refactor code for returning error message The idea now is to rely more on `net/http` and less on our own internal table. * chore: major code refactoring This essentially separates the actual API call from the translator bits, so we can now work on the extra nifty features we need. * feat: add simple function to return usage * feat: adding usage call * chore: refactor code to avoid object ambiguities New code requires passing structs representing JSON objects, instead of relying on loose interface conversions which may fail. Stricter is better! * chore: add help for languages * fix: add = to flag `type` usage line * feat: adding autocomplete files * doc: mentioned the autocompletion feature * fix: correctly display versions and build dates Note: I don’t know where the “builtBy” parameter comes from; currently, it needs to be force-pushed at bildtime with a -X tag to the linker. * chore: refactor more code, add option for glossary This was mostly meant as an experiment which can later be copied & pasted for other very similar options. apiCall() gained a new parameter, the method (because some things in the API stupidly use GET and not POST) * docs: better organise the explanatons, add links * chore: refactoring code — moving setup to “Before” * feat: add more flag support, upgrade dependencies * fix: revert changes: init must be done in main() I’ve attempted to do all initialisation chores under the “Before:” for the main cli.App loop. However, this wasn’t retrieving the data properly. Moving everything back to where it was in main(). * feat: major refactoring, we’ll get rid of settings In essence, we can use and reuse the DeepLClient type/object as the ‘de facto’ settings structure, we just need to find a way to read/save settings (possibly with cli-altsrv) * chore: adding ChatGPT-generated texts for testing * feat: adding support for (simple) debugging * test: test data in XML * docs: update README with latest changes * add readline package * fix: interactive prompt now works with readline * Update README.md Correction submitted by @coderabbitai Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore: upgrade to latest versions yadda yadda * Bug: fix expected error text for DEEPL_TOKEN I had changed the text in main.g0, but forgot to update it in main_test.go * Docs: add backticks on comments * Docs: changes suggested by @coderabbitai * Docs: comments ending with period * Fix: match correct error text (changed on main.go) * Bug: possible scoping issues with deeplToken (?) Not confirmed. But… this way we can be sure that it gets properly initialised and not scooped up into the “wrong” place… * Chore: bump year to 2024 * Fix: add timeout as per @coderabbitai suggestion * Fix: add try-catch as per @coderabbitai suggestion * Chore: add test for Exists; err.Error() is redundant … at least, when called with the text formatting functions derived from the `fmt` package. * Docs: add comment * Fix: check for edge case of empty string As suggested by @coderabbitai Also: return nil and not []string{}; we’re supposed to check for the `err` code, and nil is returned avoiding memory allocation of something that will never be used… * Chore: use http.StatusXXX instead of numbers It’s more idiomatic that way, even if not necessarily easier to read (everybody knows their HTTP error codes by heart, right? no? well, then perhaps it’s better to follow the usual practice of Go’s core developers…) * Bug: missing `)` * Bug: fix a testing bug The reason for it was that a potential JSON error was not being correctly caught; this was flagged by the test suite, and therefore I sort of fixed it. Now it correctly passes all tests it’s supposed to pass :) * Doc: fix stupid typo * Docs: add comment made by @coderabbitai Future TODO — have `Languages()` optionally reply in structured formats. * Fix: error checking for writing configuration file Caught by @coderabbitai --------- Co-authored-by: mochi-MizLab <mochice.mls.ntl@gmail.com> Co-authored-by: Osamu Takiya <takiya@toran.sakura.ne.jp> Co-authored-by: Omochice <44566328+Omochice@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
WalkthroughThis update introduces a comprehensive enhancement across the project, focusing on coding standards, GitHub workflow optimizations, expanded Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files ignored due to path filters (2)
Files selected for processing (19)
Files skipped from review due to trivial changes (3)
Additional Context UsedLearnings (2)
Additional comments not posted (35)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| type DeepL interface { | ||
| Translate(Text string, setting Setting) ([]string, error) | ||
| } | ||
|
|
||
| type DeepLClient struct { | ||
| Endpoint string | ||
| AuthKey string | ||
| } | ||
| // Now dig through settings and extract what we can... | ||
|
|
||
| func (c *DeepLClient) Translate(text string, sourceLang string, targetLang string) ([]string, error) { | ||
| params := url.Values{} | ||
| params.Add("auth_key", c.AuthKey) | ||
| params.Add("source_lang", sourceLang) | ||
| params.Add("target_lang", targetLang) | ||
| params.Add("text", text) | ||
| resp, err := http.PostForm(c.Endpoint, params) | ||
|
|
||
| if err := ValidateResponse(resp); err != nil { | ||
| return []string{}, err | ||
| var vcs, rev string // Name of the version control system name (very likely Git) and the revision. | ||
| for _, setting := range buildInfo.Settings { | ||
| switch setting.Key { | ||
| case "vcs": | ||
| vcs = setting.Value | ||
| case "vcs.revision": | ||
| rev = setting.Value | ||
| case "vcs.time": | ||
| versionInfo.dateString = setting.Value | ||
| } | ||
| } | ||
| parsed, err := ParseResponse(resp) | ||
| if err != nil { | ||
| return []string{}, err | ||
| versionInfo.commit = "unknown" | ||
| if vcs != "" { | ||
| versionInfo.commit = vcs | ||
| } | ||
| r := []string{} | ||
| for _, translated := range parsed.Translations { | ||
| r = append(r, translated.Text) | ||
| if rev != "" { | ||
| versionInfo.commit += " [" + rev + "]" | ||
| } | ||
| return r, nil | ||
| } | ||
|
|
||
| var KnownErrors = map[int]string{ | ||
| 400: "Bad request. Please check error message and your parameters.", | ||
| 403: "Authorization failed. Please supply a valid auth_key parameter.", | ||
| 404: "The requested resource could not be found.", | ||
| 413: "The request size exceeds the limit.", | ||
| 414: "The request URL is too long. You can avoid this error by using a POST request instead of a GET request, and sending the parameters in the HTTP body.", | ||
| 429: "Too many requests. Please wait and resend your request.", | ||
| 456: "Quota exceeded. The character limit has been reached.", | ||
| 503: "Resource currently unavailable. Try again later.", | ||
| 529: "Too many requests. Please wait and resend your request.", | ||
| } // this from https://www.deepl.com/docs-api/accessing-the-api/error-handling/ | ||
| // attempt to parse the date, which comes as a string in RFC3339 format, into a date.Time: | ||
| var parseErr error | ||
| if versionInfo.date, parseErr = time.Parse(versionInfo.dateString, time.RFC3339); parseErr != nil { | ||
| // Note: we can safely ignore the parsing error: either the conversion works, or it doesn't, and we | ||
| // cannot do anything about it... (gwyneth 20231103) | ||
| // However, the AI revision bots dislike this, so we'll assign the current date instead. | ||
| versionInfo.date = time.Now() | ||
|
|
||
| func ValidateResponse(resp *http.Response) error { | ||
| if resp.StatusCode < 200 || resp.StatusCode >= 300 { | ||
| var data map[string]interface{} | ||
| baseErrorText := fmt.Sprintf("Invalid response [%d %s]", | ||
| resp.StatusCode, | ||
| http.StatusText(resp.StatusCode)) | ||
| if t, ok := KnownErrors[resp.StatusCode]; ok { | ||
| baseErrorText += fmt.Sprintf(" %s", t) | ||
| } | ||
| e := json.NewDecoder(resp.Body).Decode(&data) | ||
| if e != nil { | ||
| return fmt.Errorf("%s", baseErrorText) | ||
| } else { | ||
| return fmt.Errorf("%s, %s", baseErrorText, data["message"]) | ||
| if debugLevel > 1 { | ||
| fmt.Fprintf(os.Stderr, "date parse error: %v", parseErr) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| type Setting struct { | ||
| AuthKey string `json:"-"` | ||
| SourceLang string `json:"source_lang"` | ||
| TargetLang string `json:"target_lang"` | ||
| IsPro bool `json:"-"` | ||
| } | ||
| // NOTE: I have no idea where the "builtBy" info is supposed to come from; | ||
| // the way I do it is to force the variable with a compile-time option. (gwyneth 20231103) | ||
| versionInfo.builtBy = TheBuilder | ||
|
|
||
| type DeepLResponse struct { | ||
| Translations []Translated | ||
| return nil |
There was a problem hiding this comment.
Refactor initVersionInfo to improve readability and error handling.
The function initVersionInfo is quite lengthy and handles multiple aspects of version information initialization. Consider breaking it down into smaller, more focused functions. Additionally, the error handling for date parsing (lines 85-92) silently fails, which might not be ideal. Consider logging this error or handling it in a way that does not potentially hide issues.
| // TODO: Try to use "github.com/urfave/cli/v3" in the future... | ||
| // TODO: @urfave has his own library to deal with configuration files, cli-altsrc. | ||
| // It's obscure and sparsely documented (see ). | ||
| // But it's probably far more flexible than the simplistic scheme used here. (gwyneth 20231103) |
There was a problem hiding this comment.
Consider using github.com/urfave/cli/v3 for future development.
The TODO comments on lines 191-194 suggest an upgrade to cli/v3 and a switch to cli-altsrc for configuration management. If desired, I can help create a plan or a GitHub issue to track this upgrade process.
| }, | ||
| } | ||
| err := app.Run(os.Args) | ||
| err = app.Run(os.Args) |
There was a problem hiding this comment.
Ensure proper error handling in the main function.
The main function uses log.Fatal(err) for error handling, which immediately terminates the program. While this is appropriate in some cases, consider if there's a need for more graceful shutdown processes or more detailed logging before exiting.
…er things (#20)
📝 Add badge to README.md
📝 change usage text
💪 Delete
stdinflag📝 Update README
⤴ Update version of library
🥷 divide package
🐛 fix test and ci
💪 Add test
🐛 Fix default config directory permission to 0755
To avoid "permission denied"
Add renovate.json (Configure Renovate #11)
chore: use conventional commit (chore: use conventional commit #15)
Update module github.com/mattn/go-isatty to v0.0.19 (fix(deps): update module github.com/mattn/go-isatty to v0.0.19 #12)
fix(deps): update module github.com/urfave/cli/v2 to v2.25.7 (fix(deps): update module github.com/urfave/cli/v2 to v2.25.7 #13)
chore(deps): update actions/checkout action to v4 (chore(deps): update actions/checkout action to v4 #16)
chore(deps): update actions/setup-go action to v4 (chore(deps): update actions/setup-go action to v4 - autoclosed #17)
chore(deps): update goreleaser/goreleaser-action action to v5 (chore(deps): update goreleaser/goreleaser-action action to v5 #18)
build: update golang version
ci: update ci
ci: fix go-version
fix(deps): update module github.com/mattn/go-isatty to v0.0.20 (fix(deps): update module github.com/mattn/go-isatty to v0.0.20 #19)
chore: update .goreleaser
build: update go.sum
Chore: small fixes Mostly English spelling and getting rid of deprecated Go functions.
chore: refactor code for returning error message The idea now is to rely more on
net/httpand less on our own internal table.chore: major code refactoring This essentially separates the actual API call from the translator bits, so we can now work on the extra nifty features we need.
feat: add simple function to return usage
feat: adding usage call
chore: refactor code to avoid object ambiguities New code requires passing structs representing JSON objects, instead of relying on loose interface conversions which may fail. Stricter is better!
chore: add help for languages
fix: add = to flag
typeusage linefeat: adding autocomplete files
doc: mentioned the autocompletion feature
fix: correctly display versions and build dates
Note: I don’t know where the “builtBy” parameter comes from; currently, it needs to be force-pushed at bildtime with a -X tag to the linker.
chore: refactor more code, add option for glossary This was mostly meant as an experiment which can later be copied & pasted for other very similar options. apiCall() gained a new parameter, the method (because some things in the API stupidly use GET and not POST)
docs: better organise the explanatons, add links
chore: refactoring code — moving setup to “Before”
feat: add more flag support, upgrade dependencies
fix: revert changes: init must be done in main() I’ve attempted to do all initialisation chores under the “Before:” for the main cli.App loop. However, this wasn’t retrieving the data properly. Moving everything back to where it was in main().
feat: major refactoring, we’ll get rid of settings In essence, we can use and reuse the DeepLClient type/object as the ‘de facto’ settings structure, we just need to find a way to read/save settings (possibly with cli-altsrv)
chore: adding ChatGPT-generated texts for testing
feat: adding support for (simple) debugging
test: test data in XML
docs: update README with latest changes
add readline package
fix: interactive prompt now works with readline
Update README.md
Correction submitted by @coderabbitai
chore: upgrade to latest versions yadda yadda
Bug: fix expected error text for DEEPL_TOKEN I had changed the text in main.g0, but forgot to update it in main_test.go
Docs: add backticks on comments
Docs: changes suggested by @coderabbitai
Docs: comments ending with period
Fix: match correct error text (changed on main.go)
Bug: possible scoping issues with deeplToken (?) Not confirmed. But… this way we can be sure that it gets properly initialised and not scooped up into the “wrong” place…
Chore: bump year to 2024
Fix: add timeout as per @coderabbitai suggestion
Fix: add try-catch as per @coderabbitai suggestion
Chore: add test for Exists; err.Error() is redundant … at least, when called with the text formatting functions derived from the
fmtpackage.Docs: add comment
Fix: check for edge case of empty string As suggested by @coderabbitai
Also: return nil and not []string{}; we’re supposed to check for the
errcode, and nil is returned avoiding memory allocation of something that will never be used…Chore: use http.StatusXXX instead of numbers It’s more idiomatic that way, even if not necessarily easier to read (everybody knows their HTTP error codes by heart, right? no? well, then perhaps it’s better to follow the usual practice of Go’s core developers…)
Bug: missing
)Bug: fix a testing bug The reason for it was that a potential JSON error was not being correctly caught; this was flagged by the test suite, and therefore I sort of fixed it. Now it correctly passes all tests it’s supposed to pass :)
Doc: fix stupid typo
Docs: add comment made by @coderabbitai Future TODO — have
Languages()optionally reply in structured formats.Fix: error checking for writing configuration file Caught by @coderabbitai
Summary by CodeRabbit
New Features
deeplpackage for interacting with the DeepL translation API.Documentation
Bug Fixes
.gitignoreto exclude more files such as macOS temporary files and editor-related files, improving project cleanliness.Chores
.editorconfigto enforce consistent coding styles.renovate.jsonfor automated dependency updates.Refactor
main.gofor enhanced functionality and error handling.