Drifty icon indicating copy to clipboard operation
Drifty copied to clipboard

feat: add command line URL queue

Open Femi-lawal opened this issue 1 year ago • 25 comments

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: image

Add: image

Remove: image

List: image

Get: image

Summary by CodeRabbit

  • New Features
    • Enhanced CLI functionality to manage URLs, including addition, listing, removal, and batch downloading from a YAML file.

Femi-lawal avatar Jan 15 '24 12:01 Femi-lawal

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

vercel[bot] avatar Jan 15 '24 12:01 vercel[bot]

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?

Share
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.

coderabbitai[bot] avatar Jan 15 '24 12:01 coderabbitai[bot]

@Femi-lawal Awesome work :clap:. Some minor changes are required. Will let you know tomorrow :grin:. Thank you for the PR!

SaptarshiSarkar12 avatar Jan 15 '24 17:01 SaptarshiSarkar12

: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]

github-actions[bot] avatar Jan 20 '24 14:01 github-actions[bot]

: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]

github-actions[bot] avatar Jan 26 '24 07:01 github-actions[bot]

@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 👇 image

@SaptarshiSarkar12 I've fixed the issue

Femi-lawal avatar Jan 26 '24 07:01 Femi-lawal

@SaptarshiSarkar12 I've fixed the issue

Great :+1:! It's working now.

SaptarshiSarkar12 avatar Jan 28 '24 11:01 SaptarshiSarkar12

@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 and get 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 that

    No URL is present in the links queue!
    Please run with `add <link>` to add the link to the list.
    

    image

  • 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 like remove all to remove all the links. What do you think?

@SaptarshiSarkar12 Thanks. I've implemented the changes.

Femi-lawal avatar Jan 28 '24 18:01 Femi-lawal

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.

SaptarshiSarkar12 avatar Jan 30 '24 06:01 SaptarshiSarkar12

@Femi-lawal Please make the requested changes 😃.

SaptarshiSarkar12 avatar Feb 22 '24 12:02 SaptarshiSarkar12

: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]

github-actions[bot] avatar Feb 23 '24 19:02 github-actions[bot]

@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.

SaptarshiSarkar12 avatar Mar 02 '24 18:03 SaptarshiSarkar12

: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]

github-actions[bot] avatar Mar 03 '24 22:03 github-actions[bot]

:x: Linting errors found!

@Femi-lawal Please fix the following errors:


github-actions[bot] avatar Mar 03 '24 23:03 github-actions[bot]

:x: Linting errors found!

@Femi-lawal Please fix the following errors:


github-actions[bot] avatar Mar 04 '24 00:03 github-actions[bot]

: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]

github-actions[bot] avatar Mar 04 '24 00:03 github-actions[bot]

@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 add Environment.terminate(statusCode) in the listUrls method with the status code as 0 for no issues and 1 if any exception or error has been encountered. image
  • If only 1 url exist, the print message does not sound good as shown below 👇. Can you please fix this? image

I've added the exit codes

Femi-lawal avatar Mar 16 '24 10:03 Femi-lawal

@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).

SaptarshiSarkar12 avatar Mar 16 '24 10:03 SaptarshiSarkar12

@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 avatar Mar 16 '24 10:03 Femi-lawal

@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).

SaptarshiSarkar12 avatar Mar 16 '24 10:03 SaptarshiSarkar12

@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).

I've made the change

Femi-lawal avatar Mar 16 '24 16:03 Femi-lawal

@Femi-lawal Sorry for the delay in reviewing this PR. I was busy the last week. I will review this now.

SaptarshiSarkar12 avatar Mar 23 '24 04:03 SaptarshiSarkar12

@Femi-lawal Please make the requested changes 😁.

SaptarshiSarkar12 avatar Apr 11 '24 03:04 SaptarshiSarkar12

@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) : 
    

    image

@SaptarshiSarkar12 Currently if you intensionally try to add a URL with white spaces it throws an error, which I think is fine image

I think the amount of code required just to give a better experience for this niche example is unjustified image

The error handling for Utility.parseStringToInt() doesn't work well with the existing validation, which works fine. image

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 avatar Apr 21 '24 14:04 Femi-lawal

@Femi-lawal It's fine. No problem. I can make the last required changes and merge this PR. Thank you for your contributions :smile:!

SaptarshiSarkar12 avatar Apr 22 '24 19:04 SaptarshiSarkar12

: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]

github-actions[bot] avatar May 04 '24 14:05 github-actions[bot]