-
-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update code to support TA - Tamil language #33
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,7 +51,15 @@ pub struct Cli { | |
#[clap( | ||
long = "update-langs", | ||
help = "Update all language files", | ||
long_help = "Update all language files from the repository https://github.com/hisbaan/wordlists." | ||
long_help = "Update all language files from the repository specified by CLI @wordlist-url@." | ||
)] | ||
pub update_langs: bool, | ||
#[clap( | ||
short = 'w', | ||
long = "wordlist-url", | ||
help = "Wordlist repository URL", | ||
long_help = "Wordlist repository URL. The default value is 'https://raw.githubusercontent.com/hisbaan/wordlists/main'", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rust has a sensible affordance here for CLI help to also print the default; this is not the case in C++, Python @ argparse etc. nice to know! |
||
default_value = "https://raw.githubusercontent.com/hisbaan/wordlists/main" | ||
)] | ||
pub wordlist_url: String, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ pub fn yank(string: &str) { | |
"freebsd", | ||
"netbsd", | ||
"dragonfly", | ||
"mac osx", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason "mac osx" was not on this list before is that (as far as I know) it handles clipboard functionality just fine without this hack. This is not a "supported operating systems" list but instead a list of operating systems for which a certain workaround is required. Please remove it from this list. |
||
"netbsd", | ||
"openbsd", | ||
"solaris", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,7 +58,11 @@ fn run_app() -> std::result::Result<(), Error> { | |
|
||
// Update all downloaded languages. | ||
if args.update_langs { | ||
update_langs(); | ||
assert!( | ||
!args.wordlist_url.ends_with("/"), | ||
"URL should end with branch name in github without trailing /" | ||
); | ||
Comment on lines
+61
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine, but please use the proper error handling instead of just asserting. You can see an example a little below in the code or right here: let error = cmd.error(
clap::ErrorKind::MissingRequiredArgument,
format!(
"The {} argument was not provided.\n\n\tEither provide it as an argument or pass it in from standard input.",
"<SEARCH_TERM>".green()
)
);
clap::Error::exit(&error); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should also note that the repository doesn't necessarily have to be on GitHub so the error message should be something like
|
||
update_langs(args.wordlist_url); | ||
std::process::exit(0); | ||
} | ||
|
||
|
@@ -88,7 +92,7 @@ fn run_app() -> std::result::Result<(), Error> { | |
} | ||
|
||
if SUPPORTED_LANGS.contains_key(args.lang.as_str()) { | ||
fetch_word_list(args.lang.to_owned()); | ||
fetch_word_list(args.lang.to_owned(), args.wordlist_url.to_owned()); | ||
} else { | ||
// Not supported | ||
// Initialize new command. | ||
|
@@ -110,14 +114,13 @@ fn run_app() -> std::result::Result<(), Error> { | |
// Exit with error. | ||
clap::Error::exit(&error); | ||
} | ||
|
||
// Get word list. The program will only get here if/when this is a valid word list. | ||
let word_list = read_to_string(dirs::data_dir().unwrap().join("didyoumean").join(args.lang)) | ||
.expect("Error reading file"); | ||
|
||
// Get dictionary of words from words.txt. | ||
let dictionary = word_list.split('\n'); | ||
|
||
//assert!(dictionary.clone().count()>20,"Size of wordlist > 20 words"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was used in debugging? Please remove this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. still new to Rust as you can see from the clone-ing. Sorry. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No worries! I really appreciate the effort and desire to make software better :) |
||
// Create mutable vecs for storing the top n words. | ||
let mut top_n_words = vec![""; args.number]; | ||
let mut top_n_dists = vec![search_term.len() * 10; args.number]; | ||
|
@@ -224,7 +227,7 @@ fn run_app() -> std::result::Result<(), Error> { | |
/// | ||
/// * `lang` - A locale code string to define the word list file to fetch. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please update the comment string here with the new parameter. |
||
#[tokio::main] | ||
async fn fetch_word_list(lang: String) { | ||
async fn fetch_word_list(lang: String, wordlist_url: String) { | ||
// Get data directory. | ||
let data_dir = dirs::data_dir().unwrap().join("didyoumean"); | ||
|
||
|
@@ -243,11 +246,8 @@ async fn fetch_word_list(lang: String) { | |
LOCALES.get(&lang).unwrap().to_string().blue() | ||
); | ||
|
||
let url = format!( | ||
"https://raw.githubusercontent.com/hisbaan/wordlists/main/{}", | ||
&lang | ||
); | ||
|
||
let url = format!("{}/{}", wordlist_url, &lang); | ||
println!("Accessing URL: {}", url); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This extra print is not required as it adds extra clutter and doesn't give any new information to the user since they know the URL they added. |
||
// Setup reqwest. | ||
let response = get(&url).await.expect("Request failed"); | ||
let total_size = response.content_length().unwrap(); | ||
|
@@ -280,7 +280,7 @@ async fn fetch_word_list(lang: String) { | |
} | ||
|
||
/// Update the word list files by deleting and downloading the files from the repository. | ||
fn update_langs() { | ||
fn update_langs(wordlist_url: String) { | ||
let data = data_dir().unwrap().join("didyoumean"); | ||
|
||
// Create data directory if it doesn't exist. | ||
|
@@ -299,7 +299,7 @@ fn update_langs() { | |
// Only delete and download if the language is supported. | ||
if SUPPORTED_LANGS.contains_key(string) { | ||
remove_file(data.join(&string)).expect("Failed to update file (deletion failed)"); | ||
fetch_word_list(string.to_string()); | ||
fetch_word_list(string.to_string(), wordlist_url.to_string()); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove these changes to the README.