Drifty
Drifty copied to clipboard
feat: add command line URL queue
Fixes issue
Fixes #410
Changes proposed
- Introduced a new command
add
in the CLI to add URLs to a local text file for batch processing. - Implemented a
list
command to display all the URLs currently stored in the text file. - Implemented a
remove
command to remove the URL at the specified line number. - Implemented a
get
command to start downloading the contents of the text file.
Check List (Check all the applicable boxes)
- [x] My code follows the code style of this project.
- [ ] My change requires changes to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] This PR does not contain plagiarized content.
- [x] The title of my pull request is a short description of the requested changes.
Screenshots
Help:
Add:
Remove:
List:
Get:
Summary by CodeRabbit
-
New Features
- Enhanced CLI functionality to manage URLs, including addition, listing, removal, and batch downloading from a YAML file.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
drifty | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | May 6, 2024 10:13am |
Walkthrough
The recent update to the Drifty CLI application enhances its functionality by allowing users to manage URLs through a command-line interface. This update enables users to add, list, remove URLs from a download queue, and perform batch downloads, leveraging a YAML file for efficient data storage and retrieval.
Changes
File Path | Change Summary |
---|---|
CLI/src/main/java/main/Drifty_CLI.java |
Updated to manage URLs via CLI, including addition, removal, listing, and batch downloading using a YAML file. |
CLI/src/main/java/cli/support/Constants.java |
Updated with new flags and messages for CLI operations. |
CLI/src/main/java/cli/utils/Utility.java |
Updated with new imports, static imports, and a method for YAML parsing initialization. |
Assessment against linked issues
Objective | Addressed | Explanation |
---|---|---|
Add links to a download queue via CLI (#410) | ✅ | |
List URLs in the queue (#410) | ✅ | |
Remove specific URLs from the queue by line number (#410) | ✅ | |
Batch download URLs from the queue (#410) | ✅ |
Poem
🐇 In the CLI's realm, where URLs flow,
Drifty now manages them with a new glow.
Add, list, remove, a queue's delight,
Batch download awaits, in the code's flight.
🌟 With each command, Drifty's power shows,
Through lines of code, where efficiency grows. 🌟
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?
Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-
I pushed a fix in commit <commit_id>.
-
Generate unit testing code for this file.
-
Open a follow-up GitHub issue for this discussion.
-
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitai
in a new review comment at the desired location with your query. Examples:-
@coderabbitai generate unit testing code for this file.
-
@coderabbitai modularize this function.
-
- PR comments: Tag
@coderabbitai
in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:-
@coderabbitai generate interesting stats about this repository and render them as a table.
-
@coderabbitai show all the console.log statements in this repository.
-
@coderabbitai read src/utils.ts and generate unit testing code.
-
@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
-
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)
-
@coderabbitai pause
to pause the reviews on a PR. -
@coderabbitai resume
to resume the paused reviews. -
@coderabbitai review
to trigger a review. This is useful when automatic reviews are disabled for the repository. -
@coderabbitai resolve
resolve all the CodeRabbit review comments. -
@coderabbitai help
to get help.
Additionally, you can add @coderabbitai ignore
anywhere in the PR description to prevent this PR from being reviewed.
CodeRabbit Configration File (.coderabbit.yaml
)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yaml
file to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
@Femi-lawal Awesome work :clap:. Some minor changes are required. Will let you know tomorrow :grin:. Thank you for the PR!
:x: Linting errors found!
@Femi-lawal Please fix the following errors:
CLI/src/main/java/main/Drifty_CLI.java:80:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:96:34: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:80:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:96:34: Control variable 'i' is modified. [ModifiedControlVariable]
:x: Linting errors found!
@Femi-lawal Please fix the following errors:
CLI/src/main/java/main/Drifty_CLI.java:69:52: Use a single space to separate non-whitespace characters. [SingleSpaceSeparator]
CLI/src/main/java/main/Drifty_CLI.java:81:52: Use a single space to separate non-whitespace characters. [SingleSpaceSeparator]
CLI/src/main/java/main/Drifty_CLI.java:69:52: Use a single space to separate non-whitespace characters. [SingleSpaceSeparator]
CLI/src/main/java/main/Drifty_CLI.java:81:52: Use a single space to separate non-whitespace characters. [SingleSpaceSeparator]
@Femi-lawal I am getting error when I run Drifty CLI with the argument
add https://www.youtube.com/watch?v=BB5jK8CdAno&t=921s
as you can see in the below screenshot 👇
@SaptarshiSarkar12 I've fixed the issue
@SaptarshiSarkar12 I've fixed the issue
Great :+1:! It's working now.
@Femi-lawal Please make the listed changes 👇
After all the files are downloaded from the list, the links yml file is not cleared and/or deleted. So, this causes downloading the same file again.
If
links.yml
file does not exist andget
argument is passed to Drifty CLI, it creates a new yml file and then, checks if any url is there --- which is redundant. Instead, it should check if yml file exists, if it exists - it will download the files and if it does not, then, it will print thatNo URL is present in the links queue! Please run with `add <link>` to add the link to the list.
When URLs are added/removed/get, the list of URLs are also displayed which is simply redundant usage of
list
argument.Please add a feature for adding multiple URLs which must be whitespace separated like this 👇
add <link_1> <link_2> ...
Similarly, for remove argument, it can be made like this 👇
remove 1 2 3 ...
Also, update the
help()
method to reflect the changes you are going to make 😄.@Femi-lawal I have an idea - can we add a special word after
remove
argument likeremove all
to remove all the links. What do you think?
@SaptarshiSarkar12 Thanks. I've implemented the changes.
Thank you for making those changes 😁. I will again be out of station for some days and hopefully, I'll return by 13th February 😀. Will check after that.
@Femi-lawal Please make the requested changes 😃.
:x: Linting errors found!
@Femi-lawal Please fix the following errors:
CLI/src/main/java/main/Drifty_CLI.java:299:9: 'if' construct must use '{}'s. [NeedBraces]
CLI/src/main/java/main/Drifty_CLI.java:314:9: 'if' construct must use '{}'s. [NeedBraces]
CLI/src/main/java/main/Drifty_CLI.java:329:9: 'if' construct must use '{}'s. [NeedBraces]
CLI/src/main/java/main/Drifty_CLI.java:119:12: 'for rcurly' has incorrect indentation level 11, expected level should be 12. [Indentation]
CLI/src/main/java/main/Drifty_CLI.java:281:9: 'if' construct must use '{}'s. [NeedBraces]
@Femi-lawal Please make the requested changes after resolving the branch conflicts.
Please remember that you must not remove any changes (that are out of context of this issue) from the
master
branch during resolving the conflict.
:x: Linting errors found!
@Femi-lawal Please fix the following errors:
CLI/src/main/java/main/Drifty_CLI.java:99:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:108:30: Control variable 'i' is modified. [ModifiedControlVariable]
:x: Linting errors found!
@Femi-lawal Please fix the following errors:
:x: Linting errors found!
@Femi-lawal Please fix the following errors:
:x: Linting errors found!
@Femi-lawal Please fix the following errors:
CLI/src/main/java/main/Drifty_CLI.java:632:9: Unused local variable 'yaml'. [UnusedLocalVariable]
@Femi-lawal Along with the above requested changes, please take a look at the below issues 👇
- When I pass the
list
command, it gives an error after printing the URLs. I suggest you addEnvironment.terminate(statusCode)
in thelistUrls
method with the status code as0
for no issues and1
if any exception or error has been encountered.![]()
- If only 1 url exist, the print message does not sound good as shown below 👇. Can you please fix this?
![]()
I've added the exit codes
@Femi-lawal I think it would be better to add the terminate
code in the method so that we can use exit code 1 if any error occurs, else 0. What do you think?
Exit codes are very important for any CLI application and must not always be zero (which means
successful execution
).
@Femi-lawal I think it would be better to add the
terminate
code in the method so that we can use exit code 1 if any error occurs, else 0. What do you think?Exit codes are very important for any CLI application and must not always be zero (which means
successful execution
).
Sorry, in which method?
@Femi-lawal I think it would be better to add the
terminate
code in the method so that we can use exit code 1 if any error occurs, else 0. What do you think?Exit codes are very important for any CLI application and must not always be zero (which means
successful execution
).Sorry, in which method?
In listUrls()
method I suppose.
You just added a commit to terminate after the listUrls
method exits. So, I asked you to add the terminate code with the correct status code according to the execution of the method block (successful or throwing an error).
@Femi-lawal I think it would be better to add the
terminate
code in the method so that we can use exit code 1 if any error occurs, else 0. What do you think?Exit codes are very important for any CLI application and must not always be zero (which means
successful execution
).Sorry, in which method?
In
listUrls()
method I suppose. You just added a commit to terminate after thelistUrls
method exits. So, I asked you to add the terminate code with the correct status code according to the execution of the method block (successful or throwing an error).
I've made the change
@Femi-lawal Sorry for the delay in reviewing this PR. I was busy the last week. I will review this now.
@Femi-lawal Please make the requested changes 😁.
@Femi-lawal Please make the three requested changes along with the below mentioned issue. Then, we can have this PR merged 🎉!
If only 1 url exist, the print message does not sound good as shown below 👇. Can you please fix this? Something like this 👇 can be added.
There's only one URL in the list! Would you like to remove it? (Y/N) :
@SaptarshiSarkar12
Currently if you intensionally try to add a URL with white spaces it throws an error, which I think is fine
I think the amount of code required just to give a better experience for this niche example is unjustified
The error handling for Utility.parseStringToInt()
doesn't work well with the existing validation, which works fine.
Unfortunately, too much time has passed, and I am very busy now. I can't carve out any more time to make significant code changes to the pull request and test them. If there are any changes you think are of utmost importance, please do you mind implementing them so the pull request can be merged? I apologize for the inconvenience, thank you.
@Femi-lawal It's fine. No problem. I can make the last required changes and merge this PR. Thank you for your contributions :smile:!
:x: Linting errors found!
@Femi-lawal Please fix the following errors:
CLI/src/main/java/main/Drifty_CLI.java:92:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:107:31: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:135:31: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:153:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:83:30: Control variable 'i' is modified. [ModifiedControlVariable]