feat: support gsub_file erroring if gsub doesn't change anything, and add gsub_file!#877
Conversation
4f92183 to
8eae746
Compare
|
@rafaelfranca can you let me know if you'd be interested in having bang methods for the other file manipulations? (either in this PR or another one) |
8eae746 to
620b685
Compare
|
@rafaelfranca could I get a re-review on this please? |
|
@rafaelfranca would you mind re-reviewing this? we'd really like to get it landed and I think I've addressed your concerns 🙂 |
54bc9de to
ae14cb1
Compare
|
@rafaelfranca @yahonda would it be possible to have this reviewed, or at least have the |
|
@rafaelfranca thanks! I have actually found it useful to have bang methods for some of the other file modification functions - would you be happy to accept a PR for those too? |
|
Yes! Happy to |
|
@rafaelfranca I've opened #908 adding a bang version of |
https://build.opensuse.org/request/show/1295381 by user dancermak + dimstar_suse - 1.4.0: ## What's Changed * Lazy-load YAML for performance improvement in rails/thor#892 * Fix encoding error when displaying diffs in rails/thor#898 * Fix unsafe shell command construction (security issue) in rails/thor#897 (bsc#1246809) * Support `git difftool`-style merge tool identifiers in rails/thor#900 * Add `gsub_file!` and make `gsub_file` fail if no substitutions occur in rails/thor#877 ## Security * CVE-2025-54314: Fixed a vulnerability where user input could result in unsafe shell command execution. (bsc#1246809) ## New Contributors * @hlascelles made their first contribution in rails/thor#893 **Full Changelog**: https://github.com/rail
🌈
This is a straw-person solution for #874 - overall I think it's not bad but expect changes to be requested; in particular, the option name is pretty verbose so happy if someone wants to suggest an alternative though to me really its about having
gsub_file!.A bonus to this is that anything using
gsub_filealso supports this behaviour such ascomment_linesanduncomment_lines- I think it would be nice to have bang versions of such methods too but want to get the core behaviour/implementation nailed down first.Resolves #874