Summary:
Deps D5198 since it uses a constant from D5198
accidentalbigdiff
I sure doubled the size of this file ^^
Could be that I added unserious business.
Test Plan:
Created a page. Had it open in two browser windows. Deleted in one. This is the second attempt:
{F34458}
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5202
Summary: Missed the "Create Document" button, thus added crumbs to Phriction Document Index
Test Plan: verified it appeared and linked accordingly
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5204
Summary:
Fixes T2208
Parent documents get created as stubs - while the action is "parent" - don't ask
Test Plan: Created some documents with somewhat ancestry - verified their parents had pages and stub messages display ({F34456}).
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2208
Differential Revision: https://secure.phabricator.com/D5201
Summary:
The actions wouldn't behave as I want. Disabled them and display "Create Document" instead, since the others make no sense on a non-existing page.
The linking was error-prone btw. `/phriction/edit//` (how it and others would be linked) wouldn't route to anything, and even if, would result in an error anyway since non-existing docs can't be deleted nor have their history shown.
Test Plan:
verified buttons disappeard when document was not created.
verified at least history button would appear when document existed, but was deleted (should work for move, too)
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2384
Differential Revision: https://secure.phabricator.com/D5203
Summary:
I missed this in review of D5155: `wordwrap()` returns a string, but `phutil_utf8_hard_wrap()` returns an array.
Implode the returned arrays so the stuff underneath it doesn't choke.
Test Plan: Clicked "show details" on a maniphest task description change
Reviewers: AnhNhan, kwadwon
Reviewed By: kwadwon
CC: aran
Differential Revision: https://secure.phabricator.com/D5195
Summary: using to phutil_utf8_hard_wrap instead of wordwrap
Test Plan: hard_wrap already unit tested
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5155
Summary: I'm too lazy to attaching them for diffs where they were introduced.
Test Plan:
/
/D1, wrote comment with code snippet
DarkConsole
commit detail, wrote comment
task detail, wrote comment
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4911
Summary:
This is pretty brutal and it adds some `phutil_safe_html()`.
But it is a big step in the right direction.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4905
Summary:
Lots of killed `phutil_escape_html()`.
Done by searching for `AphrontTableView` and then `$rows` (usually) backwards.
Test Plan:
Looked at homepage.
echo id(new AphrontTableView(array(array('<'))))->render();
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4884
Summary:
Done by searching for `AphrontDialogView` and then `appendChild()`.
Also added some `pht()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4882
Summary: Scan all phriction app files for text to pht
Test Plan: Use phriction in ALL CAPS, seems reasonably usable.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4862
Summary: Done by searching for `AphrontErrorView` and then `appendChild()`.
Test Plan:
Looked at Commit Detail.
Looked at Revision Detail.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4843
Summary: If a page has not been deleted, this adds an action button to delete the document in the menu on the Phriction Page.
Test Plan:
1. Created a document, checked whether "Delete Document" button was visible.
2. Clicked on "Delete Document" button, checked that the document had been deleted.
3. Went back to document page, checked that the "Delete Document" button no longer existed.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2385
Differential Revision: https://secure.phabricator.com/D4636
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, pht('...'))
The searched for `<` and `&` by sgrep.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4504
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, '...')
Then searched for `&` and `<` in the output and replaced them.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4503
Summary:
My claims that this worked on D4183 were incorrect:
- Make it work.
- Then use it.
- Also make cancel URI not cancel to the same page.
Test Plan: Clicked "Create Document", got a workflow on-page dialog instead of a page transition.
Reviewers: codeblock
Reviewed By: codeblock
CC: aran
Differential Revision: https://secure.phabricator.com/D4403
Summary:
Okay, just a forewarning, this is horrible right now.
But I think it's an okay enough start that it's worth sending what I've done.
This rips out the old Phriction breadcrumb UI and replaces it with the new one
that other apps have been starting to use.
Test Plan: Looked at a bunch of Phriction pages.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4183
Summary:
D4117 (which is otherwise awesome :)) requires you to `setMarkupEngine()` and
Phriction's diff rendering wasn't changed to call that with a
`PhabricatorMarkupEngine`.
Test Plan: Went to a Phriction diff page and saw it render correctly.
Reviewers: epriestley, btrahan, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4132
Summary:
AphrontSideNavView is an old class which required you to do a lot of work; it was obsoleted by AphrontSideNavFilterView. Remove all direct callsites so I can clean it up.
This is a precursor to letting me render a filter menu as a dropdown menu for T1960.
Test Plan:
Examined each interface for correct filter construction and selection:
- Browsed Diffusion
- Browsed Differential
- Browsed Files
- Browsed Slowvote
- Browsed Phriction
- Browsed repo edit interface
Grepped for `AphrontSideNavView`. The only remaining instances are in `AphrontSideNavView` itself and `AphrontSideNavFilterView` (which currently uses it).
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4034
Summary:
Some users like monospaced textareas and others don't.
This introduces an option to set this as a user preference.
Test Plan: Enabled and saw monospaced textareas, disabled and saw non-monospaced textareas.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2114
Differential Revision: https://secure.phabricator.com/D4037
Summary:
If a user is asking for a list of documents stored in Phriction, it's pretty
safe to assume that they mean documents that actually exist.
Test Plan:
Made a document, saw it listed in /phriction/list/all. Deleted it, and no longer
saw it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3951
Summary:
This doesn't actually contain any logic to prevent a user from deleting a
document twice, but it makes it significantly harder to do so.
Test Plan:
Went to edit a document, saw the delete button. Clicked it, then went back to
the edit view, and no longer saw the delete button.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3952
Summary: gives user a dialogue and they can fill out what comes after /w/
Test Plan: made some new wiki docs. played with garbage data and edit scenarios as well as vanilla create -- good ish
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1360
Differential Revision: https://secure.phabricator.com/D3946
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary:
When you delete the content of a document in Phriction, we treat it as an attempt to delete the document.
In the case you're creating the document, we hit an exception trying to delete a document which doesn't exist yet.
Detect this case and raise a better error.
Test Plan: Tried to create an empty document, got a good error. Created a nonempty document. Edited a document to empty to delete it.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1920
Differential Revision: https://secure.phabricator.com/D3728
Summary: When we receive an email, figure out if any of the other tos and ccs are users. If they are, pass their phids through the stach as "exclude phids" and exclude them from getting the email.
Test Plan: used the various applications (audit, differential, maniphest) and noted emails were sent as expected.
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: aran, Korvin, vrana
Maniphest Tasks: T1676
Differential Revision: https://secure.phabricator.com/D3645
Summary: Make these always work. Notably, this makes them work in Maniphest. Previously this was at odds with stuff fixed in D3651.
Test Plan: Dragged and dropped files into Remarkup in Maniphest.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3652
Summary: We have lots of empty drafts in DB.
Test Plan: Wrote revision comment, deleted it, checked db.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3591
Summary: ...also makes Maniphest Task Edit Controller use this when its not appropriate to upsell email.
Test Plan: played around with each tool and verified the Remarkup reference was present
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1756
Differential Revision: https://secure.phabricator.com/D3468
Summary:
I want to implement a `{P123}` rule to embed pastes, but we need viewers everywhere before it will work with privacy.
This is not exhaustive; many Remarkup callsites haven't been converted to `PhabricatorMarkupInterface` yet.
Test Plan: Looked at Maniphest, Differential, Diffusion, Phriction; added markup, made edits and hit previews.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3428
Summary:
I've replaced all `id(new PhabricatorObjectHandleData(...))->loadHandles()` by `$this->loadViewerHandles(...)`.
Lint caught one usage in a static method.
Test Plan: Displayed revision with sporadic author.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3432
Summary:
Currently, if some page /x/ has a grandchild at /x/y/z/ but /x/y/ does not exist, we insert a ghost entry for it in the "Document Hierarchy". However, we don't sort the hierarchy properly after inserting ghosts, so they always appear at the bottom and in an unuseful order.
Instead, sort children again after any ghost insertions.
Test Plan: Created /x/a/, /y/a/, /z/a/, verified ghosts sorted incorrectly before this patch and correctly afterward.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1750
Differential Revision: https://secure.phabricator.com/D3418
Summary:
- Add getHelpURI() to PhabricatorApplication for application user guides.
- Add a new "help" icon menu item and skeletal Diviner application.
- Move help tabs to Applications where they exist, document the other ones that don't exist yet.
- Grep for all tab-related stuff and delete it.
Test Plan: Clicked "help" for some apps. Clicked around randomly in a bunch of other apps.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3267
Summary:
See T1501. When users mash "save", stop them if they didn't change anything.
Also, don't default-fill the "edit notes" field with the previous notes. This is meant to be more like a commit message for your changes.
Test Plan: Edited a document with no changes, got a dialog. Edited a document with a title change only and a description change only, things worked. Edited a document with a previous "Edit notes", got a blank default fill.
Reviewers: alanh, btrahan, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1501
Differential Revision: https://secure.phabricator.com/D2978
Summary:
The immediate issue this addresses is T1366, adding a rendering cache to Phriction. For wiki pages with code blocks especially, rerendering them each time is expensive.
The broader issue is that out markup caches aren't very good right now. They have three major problems:
**Problem 1: the data is stored in the wrong place.** We currently store remarkup caches on objects. This means we're always loading it and passing it around even when we don't need it, can't genericize cache management code (e.g., have one simple script to drop/GC caches), need to update authoritative rows to clear caches, and can't genericize rendering code since each object is different.
To solve this, I created a dedicated cache database that I plan to move all markup caches to use.
**Problem 2: time-variant rules break when cached.** Some rules like `**bold**` are time-invariant and always produce the same output, but some rules like `{Tnnn}` and `@username` are variant and may render differently (because a task was closed or a user is on vacation). Currently, we cache the raw output, so these time-variant rules get locked at whatever values they had when they were first rendered. This is the main reason Phriction doesn't have a cache right now -- I wanted `{Tnnn}` rules to reflect open/closed tasks.
To solve this, I split markup into a "preprocessing" phase (which does all the parsing and evaluates all time-invariant rules) and a "postprocessing" phase (which evaluates time-variant rules only). The preprocessing phase is most of the expense (and, notably, includes syntax highlighting) so this is nearly as good as caching the final output. I did most of the work here in D737 / D738, but we never moved to use it in Phabricator -- we currently just do the two operations serially in all cases.
This diff splits them apart and caches the output of preprocessing only, so we benefit from caching but also get accurate time-variant rendering.
**Problem 3: cache access isn't batched/pipelined optimally.** When we're rendering a list of markup blocks, we should be able to batch datafetching better than we do. D738 helped with this (fetching is batched within a single hunk of markup) and this improves batching on cache access. We could still do better here, but this is at least a step forward.
Also fixes a bug with generating a link in the Phriction history interface ($uri gets clobbered).
I'm using PHP serialization instead of JSON serialization because Remarkup does some stuff with non-ascii characters that might not survive JSON.
Test Plan:
- Created a Phriction document and verified that previews don't go to cache (no rows appear in the cache table).
- Verified that published documents come out of cache.
- Verified that caches generate/regenerate correctly, time-variant rules render properly and old documents hit the right caches.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1366
Differential Revision: https://secure.phabricator.com/D2945
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.
NOTE: `arc diff` timed out so I'm pushing it without review.
Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.
Auditors: epriestley
Maniphest Tasks: T1103
Summary:
only show the blank, "create new" wiki page for the project if the project actually exists; only allow edit if the project actually exists.
Small wrinkle here is not checking if the project actually exists if the page already exists.
Test Plan:
- viewed a project wiki page
- viewed a prokect wiki page for a fake project and got a 404
- edited a project wiki page
- edited a project wiki page for a fake project and got a 404
Reviewers: epriestley, jacktrades
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1248
Differential Revision: https://secure.phabricator.com/D2506
