Guys,

since i dont have access to cvs, im posting a modified tablemanager.module.

changelog (+) for added feature (f) for bugfixes
(+) tables can be added by its name.
(+) CSV can be separeted by tradicional comma "," or semicolon ";"
(+) A .pot file has been created. Now its possible to translate the module. (im gonna post it in another issue)
(f) many texts and descriptions werent formatted as the module makers guide recommend: ALL tag were replaced by tag (fonts should be defined by themes, not modules) and many useless t() removed. It will make translations MUSH EASIER and clean.
(f) filtering using start/end now correctly eliminate the entire row.

still known issues:
* bug: Submit can delete the table when a diferent locale is on. (im working on it)
* bug: As long you can now access a table by its name, names should be unique
* bug: $atribs (the 5th parameter) should accept being NULL
* bug(?): reduce the number of strings (actually 130 entries) to make translations easier,
* feature: add more CSV separator character as "|" "/" "." or custom ones
* feature: filter the table on-the-fly (maybe thru forms) (im working on it)
* feature: import a CSV text file
* feature: import a CSV in a existing table
* feature: some massive change scripts, like put the same value to many rows, seach > check > delete, and so on. See myphpadmin for ideas.

im working in a translation to portuguese (brazil). soon i can post it to maybe include in the main package.
im working in this module coz i use this in my site. of course its not needed to be in cvs.
thats all, i think

regards to everyone

massa

CommentFileSizeAuthor
tablemanager.module85.92 KBbrmassa

Comments

brmassa’s picture

brmassa commented
Title: Some features and bugfixes » </code>stupid description

in:
(f) many texts and descriptions werent formatted as the module makers guide recommend: ALL tag were replaced by tag (fonts should be defined by themes, not modules) and many useless t() removed. It will make translations MUSH EASIER and clean.

should be:
(f) many texts and descriptions werent formatted as the module makers guide recommend: ALL FONT tag were replaced by CODE tag (fonts and colors should be defined by themes, not modules) and many useless t() removed. It will make translations MUSH EASIER and clean.

sorry,

massa

nevets’s picture

nevets commented

Regarding replacing font tags with code tags. The code tag is not standard html and relies on a drupal filter that is not normally installed or enabled. As such I do not think it should be used by default. If your object is the use of the font tag a better choice would be the span tag with a css class allowing.

brmassa’s picture

brmassa commented
Title: </code>stupid description » table manager update

netevs

the CODE tag is XHTML compliant.
the CODE tag is a drupal standard too. it is recognized by drupal core (4.5 and above) in html filtered and non filtered in ANY field.

the module help has many entries like this:
|font color='#000000'||font color='#0000BB'|<?php |/font||font color='#007700'|print |/font||font color='#0000BB'|tablemanager_display|/font||font color='#007700'|(|/font||font color='#0000BB'|1|/font||font color='#007700'|, |/font||font color='#0000BB'|35|/font||font color='#007700'|, |/font||font color='#0000BB'|TRUE|/font||font color='#007700'|); |/font||font color='#0000BB'|?>|/font||/font|

i just changed to this
|code|<?php print tablemanager_display(1, 35, TRUE); ?>|/CODE|

the themed background can be darker, or even blue. Then the help would not be seen. Once they are codes, its fair to put them between CODE tags.

regards,

massa

pobster’s picture

pobster commented
Status: Fixed » Needs work

Fantastic! :o) Nice to have another point of view regarding my module! I've only skimmed through your alterations to my code briefly but I did notice one small but glaring error so I haven't commited the changed module to the repository just yet... Afraid I *never* touch my modules over a weekend hence how comes you've not heard from me yet! ;o) But I'll take a good look tomorrow afternoon and air my views then...

...Incidentally, I did once use the <code> tag but it messes up on one of my themes and you can't read the text at all! Hence how comes I stuck with the font tags. I'll have a think about what I want to do with that as to be fair, they're only a cut and paste of the module description anyway and so aren't all that important I guess...

Pobster

pobster’s picture

pobster commented

And wow! I completely overlooked that I'd broken the 'start' 'end' function by going from array_splice to simply using temp[] = ...... It just shows how few people must use my module! ;o) Are you sure you want to help with this?!!

Pobster

brmassa’s picture

brmassa commented

Paul,

I sell customized computers in Brazil, so there are many cpu/motherboard/gpu/memory/hd combos. I use the table manager to show them all, so i want it working nicely!

1- the submit bug i need to fix, since my site isnt in english.
2- the form/filter feature: im working as a hobby, coz i dont know drupal api.
3- the CODE tag: if it is crashing, its your theme's fault hehe (in the drupal default theme it works). it helps a lot the translation work.
4- a programming hint: use L() function only when there is a translatable text. so l(%module_name) is useless. i took off all l(' '), l('%variable') entries from the code.
5- i can post the .pot file here, but it its based on the code i gave. if any l() function were changed, the .pot wont work properly.
6- it just came to me: when you put more then one start/end filter (from diferent columns), i think it wont work. It should work with any number of filters.
7- if we change the db to ensure that the table's name to be unique, a upgrade.php will be needed. but ill think about this later.

best regards,

massa

pobster’s picture

pobster commented

Hi again!

Thank you for your comments and code suggestions, most of which have been VERY helpful - especially regarding the useless t() calls, I confess that I didn't really know what they did particularly when I first started this module and I've just never bothered to clean them up!!! ;o)
I've had a chance this week to go over your changes and other suggestions and it's both good and bad...

First off, whilst I thank you greatly for pointing out the start/ end function had been broken by a code change - your fix doesn't work... In fact I'd go so far as to say it's worse than it was before!!! ;o) Whilst I see where you were going with the idea, I kinda get the feeling you perhaps didn't test it? Anyways, fixing it is next on my agenda as I've just finished the latest update which is adding tablemanager_display as a Drupal filter.

I noticed that twice (well, I only noticed two but I didn't go through every line you'd changed) you've not written a t() call correctly. The first that I found was this:

return t('Choose from the following available tables: %table',tablemanager_nodeadd());

Which should look more like this:

return t('Choose from the following available tables: %tables', array('%tables' => tablemanager_nodeadd()));

But noted that I messed up in the first place by not doing that!!! ;o) I've changed it in my own code now, thank you! I think that's another case of, yes you spotted that what I did was wrong, but you then didn't test your own code...?

Anyways, my views on your other points:

I can't see any good coming out of allowing a user to display a table by name... I think you're losing the point that tablemanager is an admin tool, not a user tool - whilst it has developed into something more than I ever hoped (or wanted) it to be - it still remains an administrative tool and as such doesn't need a pointless feature like this? I can just see an issue where two people want to call their tables the same thing and one of them will be disappointed? Far better to keep the table name and tid separate so that you can call your tables whatever you like? I see no benefit from this idea - unless anyone posts to the contrary this will not get incorporated.

$attrib being NULL... This is the LAST call so I don't see why this is an issue? If you want no attributes then you simply don't specify any? Maybe I'm missing a point with this one? But that suggestion is errr... quite frankly... stupid?

The CSV suggestions (and changes) you've made are FANTASTIC!!! :o) I'm going to incorporate them into the module once I've ironed out an issue you've created with your code (ie. your code won't always work in a certain situation) but once I'm done with it - yes, it'll definitely be included. Thanks for the ideas!

I think that's about it?

Thanks again

Pobster

brmassa’s picture

brmassa commented
Assigned: brmassa » Unassigned

paul,

ahhahah man... i sent the wrong file! hahah it was a copy that use for testing thinks. ok now, lets talk:

1- the correct code (which one i currently use) corrects this. But all my files are so modified that i need to see now what i did. But i remember the principle: unset TEMP if ANY of the cells falls into filter (ie using a boolean).
2- $attrib been NULL: first, i use one more parametre (my tests about filters), so i coded to accept null. second, error handling its a good practice. not really important indeed.
3- call tables by name: first, its more user friendly (people dont need to know the table code). second, once CSV cant be used to UPDATE or be added in a existing table, i oftem delete the old table and create a new one, with the same name. OK, can be a really mess calling two tables with same name. but its for me still a issue (like request feature) that we should think about.
4- CSV: better than displaying the pre-defined characters, is changing the "select" to a "textfield" and let users define the separators.
5- CSV UPDATE/ADD and filters: im gonna work on this coz i need. soon or later...

ps: in my todo list, i need first work on e-commerce module to fix some bugs that my clients are getting, then i come back here.

regards,

massa

pobster’s picture

pobster commented

Hello again! :o)

1. I actually made a release last night so the start/ end function is fixed now (based on your work) - so no need to worry about that part.
2. Meh, I guess it's one line to test if $attrib is NULL and change it to an empty array... I'll bung it in the next release, no worries.
3. Again, see you're saying 'user friendly' - I view this module as an admin tool and so I'm not interested in being all that user friendly... That's stepping over a line of usefulness as far as I can see? Maybe I'm blinded by my own purposes for this module, but I'll certainly need more than one persons opinion to convince me it's a good enough idea for me to include it...
4. YES!!! I was never going to incorporate your original code for CSV import. My goal was to have it user specified and I'm going to finish this right after I'm finished with the filters which currently only go so far as tid, pagination and administrative links. I'm nearly there now though, never used perl regular expressions before so this is all new to me, but I'm getting it done.
5. Not sure what you meant by this? I'm assuming you mean about adding to existing tables using csv import, which yes is a nice feature and will get included if either you or myself get round to it.

Thanks

Pobster

pobster’s picture

pobster commented

Okay as of 1.65 the non-English bug with editing tables is now fixed and also as promised, you can now pass null or whatever you like to either the sorting parameter or the attributes parameter and it gets changed into a blank array ()

Ta ;o)

Pobster

pobster’s picture

pobster commented

As of 1.68 you can now specify a separator for csv.

BTW, the code in your module was broken here (as well as in other places!) and so I'm going to close this ticket to prevent anyone downloading it.

Pobster

pobster’s picture

pobster commented
Status: Needs work » Closed (fixed)