add-random-string-to-filename.js #3
Conversation
Added functionality to custom filename template. Can now append a random string to filenames to avoid filename conflicts. Functionality should work across all platforms. String is 10 characters long and composed of alphanumerical characters only. Length of string is specified with the const "randLength". Use {rand} to specify where in the filename your random string will go.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe templating in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @Extension/Content.js:
- Around line 179-188: The sanitization step at the end of getRand is dead code
because chars only contains alphanumeric characters; remove the
.replace(/[,\\/:?<>\t\n\v\f\r]/g, '_') call and simply return randString
directly from the getRand function (ensure getRand still returns the generated
string and nothing else is relying on the replaced characters).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Extension/Content.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (3)
Extension/Content.js (3)
7-7: LGTM!The constant placement is appropriate and the value of 10 provides sufficient entropy for avoiding filename collisions (62^10 ≈ 8.4 × 10^17 possible combinations).
100-100: LGTM!The documentation clearly describes the new placeholder and follows the established format.
167-167: LGTM!The template replacement correctly generates one random string that replaces all
{rand}occurrences in the template, ensuring consistency within a single filename.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="Extension/Content.js">
<violation number="1" location="Extension/Content.js:187">
P2: The `replace` call is dead code. Since `chars` only contains alphanumeric characters (`a-zA-Z0-9`), the generated `randString` will never contain any of the characters in the regex pattern (`[,\\/:?<>\t\n\v\f\r]`). This replace operation will never match anything and can be removed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Extension/Content.js
Outdated
| randString += chars.charAt(Math.floor(Math.random() * chars.length)); | ||
| } | ||
|
|
||
| return randString.replace(/[,\\/:?<>\t\n\v\f\r]/g, '_'); |
There was a problem hiding this comment.
P2: The replace call is dead code. Since chars only contains alphanumeric characters (a-zA-Z0-9), the generated randString will never contain any of the characters in the regex pattern ([,\\/:?<>\t\n\v\f\r]). This replace operation will never match anything and can be removed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Extension/Content.js, line 187:
<comment>The `replace` call is dead code. Since `chars` only contains alphanumeric characters (`a-zA-Z0-9`), the generated `randString` will never contain any of the characters in the regex pattern (`[,\\/:?<>\t\n\v\f\r]`). This replace operation will never match anything and can be removed.</comment>
<file context>
@@ -172,6 +176,17 @@ function getLinksArr() {
+ randString += chars.charAt(Math.floor(Math.random() * chars.length));
+ }
+
+ return randString.replace(/[,\\/:?<>\t\n\v\f\r]/g, '_');
+};
+
</file context>
| return randString.replace(/[,\\/:?<>\t\n\v\f\r]/g, '_'); | |
| return randString; |
✅ Addressed in d8f7b08
The sanitization regex /[,\\/:?<>\t\n\v\f\r]/g can never match any characters in the generated string since chars only contains [A-Za-z0-9]. Dead code removed.
|
Hello @square-of-light! Thanks for this, I had reviewed it and I'm impressed with the style mimicry; couldn't have done it better myself. Yet, I want to clarify things about the use-case; what's exactly the problem? Why is this random part needed? The whole point of the extension is to avoid un-descriptive tags. Where are you saving from that resulting files are not unique? Instagram? Tumblr? |
|
Hey @Brawlence. I'm saving images off of twitter and bluesky. Those sites don't have tag functionality, so making filenames unique is difficult. Of course, this was before I was aware of the add-ons tagging functionality. I have a bad habit of not reading things when I'm hunting for solutions to my internet problems. Sorry 'bout that. I thought about keeping this to myself, but at the time, I thought it might be helpful to someone else, and you might've been interested in it as well. You don't have to commit the changes if you don't want them in your repository. I have a tool that works pretty well for me regardless, so you won't hurt my feelings if you don't want to implement the changes. Thanks for making the add-on to begin with though. You've saved me potential hours of hand-copying usernames onto filenames. My old setup worked pretty well, but was still kinda jank. This one straight-up exceeds my expectations. |
2 participants
Added functionality to custom filename template. Can now append a random string to filenames to avoid filename conflicts. Functionality should work across all platforms. String is 10 characters long and composed of alphanumerical characters only. Length of string is specified with the const "randLength". Use {rand} to specify where in the filename your random string will go.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.