Skip to content
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

Produce summary data to feed back into --table option and clean up #13

Closed
wants to merge 3 commits into from

Conversation

tuanngocnguyen
Copy link
Contributor

No description provided.

@tuanngocnguyen tuanngocnguyen changed the title Produce summary data to feed back into --table option DRAFT: Produce summary data to feed back into --table option Sep 24, 2024
@tuanngocnguyen tuanngocnguyen force-pushed the summary_output branch 2 times, most recently from d1a256c to a161615 Compare September 24, 2024 03:31
@tuanngocnguyen tuanngocnguyen force-pushed the summary_output branch 3 times, most recently from 7724b98 to bc2b345 Compare September 25, 2024 00:09
$record = $DB->get_record($table, ['id' => $id], $column);

// Replace the text.
$text = str_replace($match, $replace, $record->$column);
Copy link
Contributor

Choose a reason for hiding this comment

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

pros and cons of using $DB->replace_all_text ? I assume using it will be faster and reduce the DB io quite a lot.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, we can't use this because we can't pass in a where clause to limit it to 1 id. But I think we should clone this function and add the filter, and make a not when we go back to upstream this into core to combine this back ine

fclose($fp);

if ($summary) {
// Read the content of the output file.
Copy link
Contributor

Choose a reason for hiding this comment

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

this all looks weird to me, whats the reason for this? we really don't want to touch the file a second time and we definitely don't want to load it all into memory at once

@tuanngocnguyen tuanngocnguyen changed the title DRAFT: Produce summary data to feed back into --table option Produce summary data to feed back into --table option and clean up Sep 25, 2024
@@ -330,8 +334,7 @@ public static function regular_expression_search(string $search, string $table,
if (!empty($stream)) {
if ($summary) {
fputcsv($stream, [
$table,
$column->name,
"$table:$column->name",
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you combine these?

@tuanngocnguyen
Copy link
Contributor Author

Hi @brendanheywood ,

I have created new PR for clean up commit.
I am closing this wr.

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