Related: #1759174: Allow to customize the number of context lines in drush features-diff. (And I tried Features Override, as mentioned there... but it didn't give me what I wanted.)
Call me crazy, but I often like my diffs side by side... so I often go to the Drupal UI instead of using 'drush fd'.
There are two things which are a bit irritating though, for repeated lists of similar array structures... (like field instances):
- The context is often too small to see where we are;
- Arrays are not indented, because the browser 'squashes' spaces.
So I hacked a little settings form with a reload button into the diff output. See if you like it.
There are probably all kinds of issues with it (among others, taking too much screen real estate... and wasting resources on submiting the form, because features_access_override_actions()
first does a whole lot of processing for nothing, before doing a drupal_goto()
). But it works, and form input (through URL arguments) is correctly validated/sanitized.
Comment | File | Size | Author |
---|---|---|---|
#25 | features-diff-settings-2052187-25.patch | 45.96 KB | Sweetchuck |
#23 | customize-diff-output-jsdifflib-2052187.jpg | 110.9 KB | nagy.balint |
#23 | customize-diff-output-global-settings-2052187.jpg | 46.29 KB | nagy.balint |
#23 | customize-diff-output-2052187.jpg | 121.86 KB | nagy.balint |
#21 | features-diff-settings-2052187-21.patch | 37.64 KB | Sweetchuck |
Comments
Comment #1
roderikAhem, wrong patch. When the 'context' argument was left out, it would be set to 0.
This new patch does not set it (i.e. it defaults to a diff variable setting, which is 2 by default).
Comment #2
mpotter CreditAttribution: mpotter commentedPlease call it "context-lines" or something other than just "context". Too many "context" terms in Drupal makes it confusing.
Also feel free to add some simple stuff to features.css to get everything all on one line (both fields and the apply button).
Comment #3
roderikCool, I didn't know if you'd like the idea or the implementation. Thanks for review/feedback lots faster than the average queue maintainer :)
I think 'lines' will do for an URL argument in this case, then. Simple CSS for adding things to one line added, too. The patch still adds the CSS for indenting conditionally in admin.inc, 'inline'.
Comment #4
roderikda dum dum
Comment #6
roderikReroll against RC3
Comment #7
mpotter CreditAttribution: mpotter commentedStill needs a bit more work I think.
1) The "lines" field needs a default. Leaving it blank and checking the "indent" checkbox and then clicking the refresh button complains when the Lines field is blank.
2) Seems like it should be getting values from $form_state['values'] rather than from $_GET[].
Comment #8
SweetchuckComment #9
SweetchuckMaybe the formatter customization could be before the foreach loop instead of in the foreach loop.
Comment #10
SweetchuckComment #11
SweetchuckComment #12
SweetchuckI have some new idea.
Comment #13
Sweetchuckjsdifflib integration https://github.com/cemerick/jsdifflib
Comment #14
SweetchuckAdmin settings.
Comment #15
SweetchuckThe huge diff table was exploded by the feature components (1 table per component) and each table goes into a separated jQuery UI Tabs.
I think the markup creation is very ugly, because it does not use any theme() function.
Comment #16
nevergone CreditAttribution: nevergone commentedI tested, I think it is good.
Comment #17
SweetchuckNotice error fix.
Comment #18
SweetchuckHere are some screenshot
https://twitter.com/andor_david/status/411988055990341633/photo/1
https://twitter.com/andor_david/status/412260712258351104/photo/1
Comment #19
SweetchuckReplace context lines textfield with a select list.
hook_requirements() to inform about the available difference viewers.
Comment #20
SweetchuckIn this patch: New configuration options for default diff provider.
Next step is: Add theme functions for diff output.
Comment #21
SweetchuckComment #22
YesCT CreditAttribution: YesCT commentedI think the issue summary needs updating, and some screenshots would be nice.
I was just wishing for a setting of the number of context lines.
Comment #23
nagy.balint CreditAttribution: nagy.balint commentedThis functionality would be very useful for me, as I use the review overrides page quite often, and this patch makes diffs easier to review by showing each category in the feature as a separate tab, and also by providing a way to select how many lines to display before and after.
I reviewed the patch and it worked fine for me, i hope this gets in the module soon.
Attached a few new images about the current patch.
Comment #24
Aron NovakA few points after my review:
It needs a proper head comment.
function features_diff_formatters($all = FALSE) {
)? It might need to specify include files in the array and one (multiple?) callback per formatter and it would be more flexible in this case. It might worth consideration.Comment #25
SweetchuckComment #26
jsibley CreditAttribution: jsibley commentedHas this died? It looks like there hasn't been any update since the patch 2 years ago.