Closed (fixed)
Project:
Autocomplete Deluxe
Version:
7.x-2.x-dev
Component:
User interface
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
13 Mar 2012 at 08:58 UTC
Updated:
22 Sep 2018 at 06:32 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
arnested commentedChanged this feature request from version 1.x to 2.x.
Being able to drag and drop the selected terms to a new order would still be awesome!
Comment #2
drupalok commentedone more that would like this feature.... oops... it was me who started the thread... lol :)
Comment #3
delphian commentedAdding a patch that defaults the sort order to the taxonomy weight.
Comment #4
dxxGood feature! :)
Comment #5
jazzdrive3 commentedHere is a patch that uses the jQuery UI sortable library. It's just a change of a few lines of code and seems to work well.
Comment #6
alison+1!!
Comment #7
alisonPatch won't apply for me via drush patch-add -- the patch file itself seems to have a little different format than what I usually see, but I'm not an expert, could just be me (mainly all the descriptive text at the top -- I don't think that's usually there?).Patch format needs some TLC -- it has the file paths, etc. from the author's own system, and descriptive text at the top that shouldn't be there (although I rather like having it so handy!).
Comment #8
LNakamura commented@alisonjo2786 - you can apply #5 using the following command (in the autocomplete_deluxe module directory):
patch -p7 < /path/to/autocomplete_deluxe-ordering-1480126-5.patch@jazzdrive3 - what is the purpose of this function declaration in autocomplete_deluxe.js in #5?
Other than the above, your patch looks (and functions) great!
Comment #9
LNakamura commentedComment #10
alisonYes, thank you, that applied perfectly -- but I don't *think* it is proper coding standards to have the top of the patch file be the way it is? Even just based on the fact that I can't apply it via
drush patch-add(drushpatches.maketool).[EDIT] And now that I re-read, the site name is in the path in the patch file ("blastr") -- that should be cleared out :)
[EDIT 2] And file perms are being set by the patch (100644).
>> I'll comment once more in a minute with a new patch suggestion.
But, I love love love that this functionality has been built, I'm seriously ecstatic :D Thank you!
Comment #11
alisonEDIT: Revised version coming in a moment.......
Comment #12
alisonTake 2 -- got the author attrib right this time (I think, lol).
(All I was trying to do is clean up the paths and a couple other tidying up things.)
Comment #13
alisonComment #14
alisonRemoved aforementioned empty function.
Comment #15
LNakamura commentedThanks, @alisonjo2786!
I tested #14 (#13), and it works great.
I think you meant to call your last interdiff "interdiff-1480126-5-13.txt", so I'm including a version with that filename and re-organizing the files a bit.
Comment #16
alison@LNakamura Yes I really did lol, and I noticed it after posting (I miscounted b/c I forgot my Issue Status Change comment), but then I saw how many notifications my comments had already generated for this thread, and didn't re-re-post haha. Thank you / you're welcome!
EDIT: Jk, i meant for it to be called *-12-14, b/c it's the interdiff of 12-14 -- the patch in #14 is an update to the patch in #12, all it does is remove the empty function (#12 is already an update to #5). Sorry!!
Comment #17
alisonOkay, one more try... this time it's straight-up interdiff with #5, instead of incremental 5-12 and 12-13 (14)... I hope this is correct! Sorry!! (FWIW I think I learned a lot in the last 24 hours haha)
Comment #18
LNakamura commented@alisonjo2786 - you're welcome :-)
So it looks like #17 is the same as #13, but you just wanted to get the numbering correct? Thanks for that.
I'm really puzzled about this interdiff, which is similar to your first one. How are you generating them? You're correct that you want to compare with #5, but the output should look like your 2nd interdiff (12 vs. 13), and the one I provided for 5 vs. 13. This is the process I follow: https://www.drupal.org/documentation/git/interdiff
Glad you're learning a lot! This is my first time co-maintaining a module (as of last Friday), so I'm learning a lot as well :-)
Comment #19
alisonYay for learning! :D
So, I've been following that documentation, and a few others in the "patching" part of d.o documentation, very closely -- I'm still missing bits sometimes, but I'm attempting to follow it to the letter. I used the
interdiffcommand, b/c my Linux OS already hadpatchutilsinstalled. I could be (and probably am!) wrong, buuuuut I think your 5-13 interdiff actually wasn't an accurate representation of the differences between #5 and #13 -- *-5-13 only showed the removal of the empty function, which is the same as #12-13, but the difference between #5 and #13 (or #17) has other stuff (fixing the paths to the module).[thinking and reading/looking and typing at the same time] But something's not quite right. I'm going to look *once more*, I may have one more version, if that's not too horribly obnoxious.
(Oh and, #17 was meant to be the same as #13 + #12)
Comment #20
LNakamura commented@alisonjo2786,
Hmm.... you might be right, but my understanding is that the interdiff should show the differences between the versions of the patch from the perspective of what the patch actually does. File paths, etc., are not relevant to what the patch does to change the source code.
I'll see if I can get an authority to weigh in here.
Comment #21
alisonSo I started fresh this time -- my interdiff is still confusing to *my* eyes, but I followed all the steps -- I think it's just little things that are different (whitespace and/or starts-and-ends of files) that are causing it to be lengthy... I think!Attached is a fresh patch and interdiff, *-5-21. I'll cross my fingers and wait :)
Comment #22
alisonOMG I created new file names but forgot to re-upload, I'm so sorry, dammit dammit dammit...
This comment and file attachments are identical to the previous, but with correct comment number.
----------------------------
So I started fresh this time -- my interdiff is still confusing to *my* eyes, but I followed all the steps -- I think it's just little things that are different (whitespace and/or starts-and-ends of files) that are causing it to be lengthy... I think!
Attached is a fresh patch and interdiff, *-5-22. I'll cross my fingers and wait :)
Comment #23
alison@LNakamura re: what the interdiff should include -- OH well that makes total sense, I didn't realize that, I was following "then run this command" steps so closely I must have missed some context info in the documentation, sorry about that.
Here's what I ran, in order, just to put it all out there :) (copied from my own piece of documentation that I created yesterday/today during this process, that's why there's extra "talking to myself" info)
Comment #24
damienmckennaJust to weigh in here - patches #14 and #17 (and 21 and 22) are identical. Also, the interdiff should only contain the differences between the two patches, so the fact that it is reverting the changes to the CSS file even though both of the actual patches contain the same changes indicates it was made incorrectly.
A tip I use for making patches - get a fresh git checkout of the branch in question (git checkout .; git checkout 7.x-2.x; git pull), download & apply the first patch (curl [patch URL] | patch -p1), confirm it applied correctly if not then reroll the patch, presuming it applied correctly stage the changes (git add *), make the additional changes, use "git diff > ~/Downloads/autocomplete_deluxe-n1480126-21.interdiff.txt" to create the interdiff and "git diff HEAD > ~/Downloads/autocomplete_deluxe-n1480126-21.patch" to create the patch.
Comment #25
alisonOkay, that makes sense.
I wonder, could it be that it thinks it's doing "new changes" because it thinks the changes are to completely different module files, b/c of the file path differences?
(I'm not saying the interdiff should be as lengthy as mine is, I'm trying to figure out why the normal
interdiffcommand isn't giving me the intended results -- see also, my sequence of commands in comment #23.)Thank you!
[UPDATE] Yeah I messed up numbering and tried to re-do the uploading/commenting, sorry if that was more problematic than helpful!
Comment #26
LNakamura commentedThanks, @DamienMcKenna!
@alisonjo2786 the
interdiffcommand has at least one option (-p N- do:interdiff --help) which would allow you to ignore things that usinggit diff(per https://www.drupal.org/documentation/git/interdiff and Damien, above) ignores without requiring options.Comment #27
LNakamura commented@alisonjo2786 - I'm uploading a new (simple) interdiff-1480126-5-22.txt and cleaning up the displayed files - I think we're good on patches and interdiffs now :-) Thanks for your contributions to this issue, and I'm glad it's been a good learning experience for you! (For me, as well!)
Comment #29
LNakamura commentedMerged - thanks, all!
Comment #30
alisonCool, thanks so much for taking the time to explain / break it down for me :) I'm also thrilled to have this functionality now! (THANK YOU @jazzdrive3 !!)
Comment #31
LNakamura commentedYou're welcome! :-)
Comment #32
jrabeemer commentedCongrats. @jazzdrive3 wrote this for Syfy and we use it without issue. :)
Comment #34
m.abdulqader commentedCan we have this for Drupal 8
Comment #35
damienmckennaPlease create a new issue for that and cross-reference this one.
Comment #36
alison@m.abdulqader FWIW, the drag-and-drop reordering works for me on 8.x -- it looks like they make commits on both versions at the same time last year:
https://www.drupal.org/commitlog/commit/16390/95cd25bf4d574dd0a581e93c58...
(Also noting it here in case anyone pops into this issue -- I came back to this issue today, it hadn't even occurred to me to try it on my D8 sites -- I hadn't thought about the issue in so long -- and I was like, oh hey, it already works..... :) )