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( | |||||
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'); | |||||
Not Done Inline ActionsShouldn'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… | |||||
Not Done Inline ActionsYes, 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; | |||||
| } | |||||
Not Done Inline ActionsOn 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`? | |||||
Not Done Inline ActionsYeah. 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… | |||||
| } | } | ||||
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.