Differential D19425 Diff 46472 src/applications/differential/storage/DifferentialDiff.php

Changeset View

Standalone View

src/applications/differential/storage/DifferentialDiff.php

Show First 20 Lines • Show All 220 Lines • ▼ Show 20 Lines	foreach ($changes as $change) {
$changeset->setFileType($change->getFileType());		$changeset->setFileType($change->getFileType());
$changeset->setMetadata($metadata);		$changeset->setMetadata($metadata);
$changeset->setOldProperties($change->getOldProperties());		$changeset->setOldProperties($change->getOldProperties());
$changeset->setNewProperties($change->getNewProperties());		$changeset->setNewProperties($change->getNewProperties());
$changeset->setAwayPaths($change->getAwayPaths());		$changeset->setAwayPaths($change->getAwayPaths());
$changeset->setAddLines($add_lines);		$changeset->setAddLines($add_lines);
$changeset->setDelLines($del_lines);		$changeset->setDelLines($del_lines);

		self::detectGeneratedCode($changeset);

$diff->addUnsavedChangeset($changeset);		$diff->addUnsavedChangeset($changeset);
}		}
$diff->setLineCount($lines);		$diff->setLineCount($lines);

$parser = new DifferentialChangesetParser();		$parser = new DifferentialChangesetParser();
$changesets = $parser->detectCopiedCode(		$changesets = $parser->detectCopiedCode(
$diff->getChangesets(),		$diff->getChangesets(),
$min_width = 30,		$min_width = 30,
▲ Show 20 Lines • Show All 579 Lines • ▼ Show 20 Lines	/* -( PhabricatorConduitResultInterface )---------------------------------- */

public function getConduitSearchAttachments() {		public function getConduitSearchAttachments() {
return array(		return array(
id(new DifferentialCommitsSearchEngineAttachment())		id(new DifferentialCommitsSearchEngineAttachment())
->setAttachmentKey('commits'),		->setAttachmentKey('commits'),
);		);
}		}

		private static function detectGeneratedCode(
		epriestleyAuthorUnsubmitted Not Done Inline Actions This doesn't feel like a great place for this code, but I expect to move it somewhere better once I can combine this copy of it with the similar code in `DifferentialChangesetParser.php`, after this has been in production for a bit and/or we do a migration and/or provide some migration tools. If I just delete the display-time version of the code right now, existing changesets will lose their "generated" behavior. epriestley: This doesn't feel like a great place for this code, but I expect to move it somewhere better…
		DifferentialChangeset $changeset) {

		$is_generated_trusted = self::isTrustedGeneratedCode($changeset);

		$changeset->setTrustedChangesetAttribute(
		DifferentialChangeset::ATTRIBUTE_GENERATED,
		$is_generated_trusted);

		$is_generated_untrusted = self::isUntrustedGeneratedCode($changeset);

		$changeset->setUntrustedChangesetAttribute(
		DifferentialChangeset::ATTRIBUTE_GENERATED,
		$is_generated_untrusted);
		}

		private static function isTrustedGeneratedCode(
		DifferentialChangeset $changeset) {

		$filename = $changeset->getFilename();

		$paths = PhabricatorEnv::getEnvConfig('differential.generated-paths');
		amckinleyUnsubmitted Not Done Inline Actions Shouldn't this be per-repo? I can imagine a scenario where a random repo actually ends up producing "generated" diffs because they happened to create a file matching `differential.generated-paths`. amckinley: Shouldn't this be per-repo? I can imagine a scenario where a random repo actually ends up…
		epriestleyAuthorUnsubmitted Not Done Inline Actions Yes, this setting is pretty limited. It was added a long time ago, and I hope to remove it once there are better tools for classifying stuff as generated. We just don't have a way to do per-repo settings yet. One problem this setting is better-than-nothing at addressing is stuff like `xcode.pbproject` or whatever the file is, which XCode makes in every XCode project, is a big XML file, changes frequently, is effectively just generated code (although: not exactly), and which users can't reasonably add `@generated` to. Patterns like `./xcode.pbproject$` are probably better fits for this setting than `/path/to/some/specific/file.json`, which will misfire across repositories. epriestley:* Yes, this setting is pretty limited. It was added a long time ago, and I hope to remove it once…
		foreach ($paths as $regexp) {
		if (preg_match($regexp, $filename)) {
		return true;
		}
		}

		return false;
		}

		private static function isUntrustedGeneratedCode(
		DifferentialChangeset $changeset) {

		if ($changeset->getHunks()) {
		$new_data = $changeset->makeNewFile();
		if (strpos($new_data, '@'.'generated') !== false) {
		return true;
		}
		}

		return false;
		}
		amckinleyUnsubmitted Not Done Inline Actions On a related note, maybe the untrusted config could come in part from `.arcconfig`? amckinley: On a related note, maybe the untrusted config could come in part from `.arcconfig`?
		epriestleyAuthorUnsubmitted Not Done Inline Actions Yeah. In the future, I expect `arc` to provide untrusted configuration in most cases (see PHI64 and connected stuff, I think). We might keep some untrusted rules on the server side, so that if you copy/paste a diff with `@generated` you still get the folding behavior, but I think there's at least an argument for getting rid of them entirely, eventually. If you're copy/pasting diffs, more sophisticated stuff like `@generated` is probably not terribly important. epriestley: Yeah. In the future, I expect `arc` to provide untrusted configuration in most cases (see PHI64…

}		}