Skip to content

Conversation

side2k
Copy link
Owner

@side2k side2k commented Nov 13, 2024

Syncing with shtab.app

Copy link

@qrilka qrilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

замечания неприниципальные, но я смотрел только PR возможно имеет смысл посмотреть всю тулзу с т.зр. какой перестройки (типа чтение из хамстера один раз)

Comment on lines 49 to 55
let mut task_id: Option<String> = None;
for extractor in extractors.into_iter() {
task_id = extractor(link.url.as_str());
if task_id != None {
break;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut task_id: Option<String> = None;
for extractor in extractors.into_iter() {
task_id = extractor(link.url.as_str());
if task_id != None {
break;
}
}
let task_id = extractors.into_iter().find_map(|extractor|extractor(link.url.as_str()));

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Спасибо, не знал про find_map()!

Comment on lines 45 to 48
if links.is_empty() {
None
} else {
let link = links[0];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if links.is_empty() {
None
} else {
let link = links[0];
let link = links.get(0)?;

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил, спасибо

run_mode: RunMode,
) {
let client = ShtabClient::new(Some(api_token));
let me = client.get_profile().await.unwrap();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вместо unwrap яб возвращал anyhow::Result<()> и использовал ? - чуть сделает код короче

src/main.rs Outdated
Comment on lines 345 to 352
let activity_id:i64 = match activity_id {
Some(activity_id) => activity_id,
None => match std::env::var("HAMCLI_SHTAB_ACTIVITY") {
Ok(activity_id_str) => activity_id_str.parse::<i64>().unwrap(),
Err(_) => panic!(
"Please specify activity id via --activity-id option or HAMCLI_SHTAB_ACTIVITY env var")
}
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let activity_id:i64 = match activity_id {
Some(activity_id) => activity_id,
None => match std::env::var("HAMCLI_SHTAB_ACTIVITY") {
Ok(activity_id_str) => activity_id_str.parse::<i64>().unwrap(),
Err(_) => panic!(
"Please specify activity id via --activity-id option or HAMCLI_SHTAB_ACTIVITY env var")
}
};
let Some(activity_id:i64) = activity_id.or(std::env::var("HAMCLI_SHTAB_ACTIVITY")).and_then(|id| id.parse<i64>().ok()) else {
panic!(
"Please specify activity id via --activity-id option or HAMCLI_SHTAB_ACTIVITY env var")
};

код не проверял

Copy link
Owner Author

@side2k side2k Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

О, спасибо, что заметил. Я здесь решил немного иначе сделать(см. 56954e2) - в cli::Commands этот параметр не является опциональным (я делал немножко на будущее, но в итоге раздумал - когда прикручу функционал получения этого айдишника, тогда и сделаю опциональным).

let local_tz = Local::now().timezone();

let mut day = from;
while day <= to {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

жалька что по ходу нельзя сделать

for day in (from..=to)

while day <= to {
println!("Processing day {}", day);
let next_day = day.checked_add_days(Days::new(1)).unwrap();
let tasks = get_tasks_with_durations(hamster_db.clone(), day, next_day, category.clone());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а зачем каждый раз открывать файл? один раз в память считать и брать оттуда не проще?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Резонно. Подумаю над рефакторингом.

Comment on lines +368 to +378
let task_id: i64 = match task_id {
Some(task_id) => task_id.parse::<i64>().unwrap(),
None => match run_mode {
RunMode::DryRun => -1,
RunMode::Normal => panic!(
"Missing task id! ({}, '{}')",
task_data.duration.as_hhmm(),
task_data.title.unwrap_or("-".to_string())
),
},
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let task_id: i64 = match task_id {
Some(task_id) => task_id.parse::<i64>().unwrap(),
None => match run_mode {
RunMode::DryRun => -1,
RunMode::Normal => panic!(
"Missing task id! ({}, '{}')",
task_data.duration.as_hhmm(),
task_data.title.unwrap_or("-".to_string())
),
},
};
let Some(task_id: i64) = task_id.map(|id|id.parse::<i64>().unwrap()).or( (run_mode==RunMode::DryRun).then_some(-1)) else {
panic!(
"Missing task id! ({}, '{}')",
task_data.duration.as_hhmm(),
task_data.title.unwrap_or("-".to_string())
),
};

покороче, но не 100% понятней, конечно

src/main.rs Outdated
);

match &run_mode {
RunMode::DryRun => println!("would add new record - {data_msg}"),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RunMode::DryRun => println!("would add new record - {data_msg}"),
RunMode::DryRun => println!("would add new record - {data_msg}"),

или

Suggested change
RunMode::DryRun => println!("would add new record - {data_msg}"),
RunMode::DryRun => println!("would add new a record - {data_msg}"),

src/main.rs Outdated
);

match &run_mode {
RunMode::DryRun => println!("would add new record - {data_msg}"),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RunMode::DryRun => println!("would add new record - {data_msg}"),
RunMode::DryRun => println!("would add new record - {data_msg}"),

или

Suggested change
RunMode::DryRun => println!("would add new record - {data_msg}"),
RunMode::DryRun => println!("would add new a record - {data_msg}"),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants