WordPress.org

Make WordPress Core

Opened 9 months ago

Last modified 3 months ago

#24908 new enhancement

Proposed new filters/hooks for revisions

Reported by: adamsilverstein Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Revisions Keywords: has-patch dev-feedback
Focuses: Cc:

Description

I would like to propose a few simple filters for the revisions and diff engine in core. in particular, I am trying to create a plugin that re-introduces features tried and dropped from (or present before) the 3.6 version of revisions.

There are two features that need simple hooks to work; without them i will be forced to bundle a modified version of the revisions/diff system or something else kludgy with my plugin, and that seems ugly.

The two features are a WYSI(closer to)WYG and a single column instead of split column view for diffs.

I am attaching a patch that introduces the filters I need, not sure i'm doing it right or used appropriate names for the filters?

The plugin (warning, work in progress) is posted here: https://github.com/adamsilverstein/RevisionsPlus

Attachments (3)

24908.diff (2.1 KB) - added by adamsilverstein 9 months ago.
introduce filters for diff html processing and show_split_view option
24908.2.diff (2.2 KB) - added by adamsilverstein 9 months ago.
add context to the process_revision_diff_html filter
24908.3.diff (3.0 KB) - added by adamsilverstein 6 months ago.
add hook docs, adjusted filter name

Download all attachments as: .zip

Change History (12)

adamsilverstein9 months ago

introduce filters for diff html processing and show_split_view option

comment:1 follow-up: DrewAPicture9 months ago

  • Cc xoodrew@… added

+1 for some filters here.

We should probably add a context arg to the htmlspecialchars filters denoting each method, such as added, deleted, or context since we're using the same filter hook in three different places.

comment:2 in reply to: ↑ 1 ; follow-up: adamsilverstein9 months ago

Replying to DrewAPicture:

+1 for some filters here.

We should probably add a context arg to the htmlspecialchars filters denoting each method, such as added, deleted, or context since we're using the same filter hook in three different places.

good idea! i was wondering about that, generally the three calls are for the same purpose, but i could definitely see plugin developers adding some specific processing for just the added, deleted or unchanged lines.

there are also probably other spots that could use filters, i will keep looking;

comment:3 in reply to: ↑ 2 ; follow-up: DrewAPicture9 months ago

Replying to adamsilverstein:

... generally the three calls are for the same purpose, but i could definitely see plugin developers adding some specific processing for just the added, deleted or unchanged lines.

It's more in the vein of "most people would probably change the filter output the same for all three, but being able to filter based on context is a nice-to-have".

adamsilverstein9 months ago

add context to the process_revision_diff_html filter

comment:4 in reply to: ↑ 3 adamsilverstein9 months ago

Replying to DrewAPicture:

Replying to adamsilverstein:

... generally the three calls are for the same purpose, but i could definitely see plugin developers adding some specific processing for just the added, deleted or unchanged lines.

It's more in the vein of "most people would probably change the filter output the same for all three, but being able to filter based on context is a nice-to-have".

good suggestion, i can already think of how i would use it! added in 24908.2.diff​.

adamsilverstein6 months ago

add hook docs, adjusted filter name

comment:6 adamsilverstein6 months ago

24908.3.diff - refreshed, add hook docs, adjusted filter name

comment:7 DrewAPicture6 months ago

Inline docs notes on 24908.3.diff:
Overall:

  • There should be an empty line after the @since lines
  • No leading slash on file paths in the duplicate comments

show_revisions_split_view filter:

  • Use a docs-specific variable to represent the boolean in the parameter line. Maybe $bool or something
  • Specify the default at the end of the parameter description: "Default true."
  • Wrap the long description after "(two columns)"

process_text_diff_html filter:

  • There are three parameters but only two are documented
  • The long description is super cryptic. I had to read it a couple of times to get it. Consider using as little jargon as possible. Also, the various contexts need more explanation.

comment:8 adamsilverstein6 months ago

Thanks for the feedback, will revise.

comment:9 adamsilverstein3 months ago

  • Summary changed from Proposed new filters for revisions to Proposed new filters/hooks for revisions
Note: See TracTickets for help on using tickets.