Fixes T1476. The body of the email should be just the output of some diff command.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- Restricted Diffusion Commit
rP2fc43598b5a3: Differential - add ability to setup "create" addresses for revisions
git diff master > text.txt; ./bin/mail receive-test --to <configured-diff-create-address> < text.txt; a diff was successfully created...! email generated had a working link to the diff.
./bin/mail receive-test --to <configured-diff-create-address> < README.md; a diff was not created as expected...! email generated had a sensical error message, telling me that the mail body should have been generated via a diff command
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- T1476
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 4176 Build 4189: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
| src/applications/differential/application/PhabricatorDifferentialApplication.php | ||
|---|---|---|
| 171–172 | this doesn't render right at the moment where its used. what's the fancy command for svn users? also, its just hg diff right? I think I've got the git diff right at least. | |
Comment Actions
- Feels a little weird to actually create a revision with no info -- maybe just create a diff, and let the user use the web link to turn it into a revision?
- Users might try to email diffs -- I could imagine possibly doing something like this, maaaybe:
$diff = null;
foreach ($mail->getAttachments() as $attachment) {
try {
$diff = create from attachment;
} catch ignore {}
}
if (!$diff) {
$diff = create from mail body;
}- Does this work OK if you send it bogus junk? I think we sorted that out but maybe worth double checking if you didn't.
| src/applications/differential/application/PhabricatorDifferentialApplication.php | ||
|---|---|---|
| 171–172 | I think this text is fine as written, all of git diff ,hg diff and svn diff do something meaningful/relevant. | |
Comment Actions
On the attachment logic, my theory is that users might send one actual attachment, but sometimes mail program send random signature files and company logos and stuff.
Comment Actions
You're spot-on with this assumptions. I work with mail servers daily and I can tell You that with attachments 90% of time even if user didn't attach anything, something's there. It might be VCard (contact data), MCS (Mandatory Company Signature), Attachments with pictures that go into signature, antivirus/antimalware scan report ("Ima antivirus and i say dis mail is gut"), public key for signing... Loads of random stuff that nobody is aware that they even exist, but when You try to parse it... It's nightmare.
Comment Actions
- change the algorithm to
- filter attachments down to just text/plain, giving an error for any filtered files
- make a diff for each file, giving an error if any file fails to make a diff
- make a diff for the mail body, giving an error if the mail body doesn't parse
- re-jigger the email formatting
- go for the fancy HTML stuff
- show a list of filename: diff_uri with (if made) Mail Body: diff_uri at the bottom
- make the errors show up at the bottom of the email