This is a very rough sketch
| Comment | File | Size | Author |
|---|---|---|---|
| #53 | interdiff-49-53.txt | 2.8 KB | MerryHamster |
| #53 | 2546280-53.patch | 6.31 KB | MerryHamster |
| #49 | 2546280-49.patch | 6.52 KB | jofitz |
| #49 | interdiff-46-49.txt | 2.08 KB | jofitz |
| #46 | 2546280-46.patch | 7.33 KB | jofitz |
This is a very rough sketch
| Comment | File | Size | Author |
|---|---|---|---|
| #53 | interdiff-49-53.txt | 2.8 KB | MerryHamster |
| #53 | 2546280-53.patch | 6.31 KB | MerryHamster |
| #49 | 2546280-49.patch | 6.52 KB | jofitz |
| #49 | interdiff-46-49.txt | 2.08 KB | jofitz |
| #46 | 2546280-46.patch | 7.33 KB | jofitz |
Comments
Comment #2
alexpottComment #4
alexpottRerolled and improved a little.
Comment #5
xjmKinda interesting, and the ability to add themeability and pass around a classed object is intriguing too, but I'm not sure about this beta-wise because it's adding new API rather than fixing what is (IMO) a blocker for and bug with
dsm()output and safeness currently.What this wouldn't cover that render arrays do is the case where you want to assemble a render array and it's not a list, e.g. appending two
t()results together in a paragraph with some whitespace or stuff with prefixes/suffixes.Comment #6
alexpottBut this should not be in a message.
Comment #7
hussainwebUber nitpicks. :)
Comment #8
xjmWhy not? Also, tell that to the code that does this. ;)
Comment #9
alexpottre #5 Well I think this API could be beta permissible as it solves the problem we have in
file_save_upload(),_node_mass_update_batch_finished(), and the example code incallback_batch_finished. AlsoConfigSync::finishBatch()could do with this tbh. Found another...update_manager_download_batch_finished- this will put the title in a<h3>in the messages block (nice).Comment #10
xjmI really don't think this is beta-eligible, unfortunately.
Comment #12
mile23Needs work in light of #2278383: Create an injectible service for drupal_set_message()
Comment #16
andypostComment #17
ashishdalviComment #18
anhtq commented(For Google Code-In)
The patch in #2 has been rerolled :)
Comment #19
chiranjeeb2410 commented@anhtq,
Patched applies cleanly. Successful reroll!
Comment #20
anhtq commentedGood to hear that !
Comment #21
mohit1604 commentedJust to inform you , drupal_set_message and drupal_get_message have been deprecated in version 8.5.x and replaced by messenger service ;)
see this for more information.
Comment #22
mohit1604 commentedAdded test to patch #18 , hope it goes green !
Comment #23
mohit1604 commentedComment #25
mohit1604 commentedMade patch for 8.4.x now working for 8.5.x according to comment #24 .
Comment #26
mohit1604 commentedComment #27
mohit1604 commenteddrupal_set_message and drupal_get_message have been deprecated in version 8.5.x (see comment #21), should we need a patch for 8.5.x for this issue ?
Comment #28
mohit1604 commentedPatch for 8.4.x , hope it shows green ;)
Comment #30
andypostPatches should be filed for current dev version
Comment #31
mohit1604 commentedPatch #28 failed testing due to following error :-
error: core/lib/Drupal/Core/Utility/Message.php: No such file or directory
error: core/modules/system/templates/status-message.html.twig: No such file or directory
These 2 files are the new files that have been added to working directory while creating this patch! I used git diff to create the patch , What should I use so that automated test can identify these as new files ?
Comment #32
chiranjeeb2410 commented@Mohit Malik, try uploading a fresh patch according to the latest version update.
Comment #33
chiranjeeb2410 commentedComment #34
chiranjeeb2410 commentedComment #36
chiranjeeb2410 commentedRerolled. Should be green.
Comment #38
chiranjeeb2410 commentedComment #40
mohit1604 commentedComment #41
mohit1604 commentedHoping to show green this time !
Comment #43
mohit1604 commentedComment #44
chiranjeeb2410 commentedComment #46
jofitzCorrect test failure.
Please remember to include an interdiff with a new patch.
Comment #48
chiranjeeb2410 commented@Jo Fitzgerald,
I think the reroll needs to be done for the patch in comment #7, since none of the patches after that have passed
automated testing.
Comment #49
jofitzRe-roll of patch from #7 (with interdiff against patch in #46).
Comment #51
dawehnerComment #52
MerryHamster commentedsorry here the wrong patch, I added the patch to #53
Comment #53
MerryHamster commentedRenamed SafeStringInterface to MarkupInterface according to https://www.drupal.org/project/drupal/issues/2576533
and https://www.drupal.org/node/2549395
Comment #54
jofitzComment #56
dawehner@alexpott Do you mind describing the intention of this?
In the case we are doing that, we also need a followup of #77245: Provide a common API for displaying JavaScript messages
Comment #67
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!