|
Bug Notes |
nepto
2003-09-20 18:55
|
Could you please post "unified diff"?
diff -Naur oldfile newfile
- oldfile is CVS version of file you tool
- newfile is file with your modifications |
|
|
michal
2003-09-22 22:38
|
i did so.
does it look reasonable and applicable ? |
|
|
nepto
2003-09-23 10:26
|
Patch looks very nice. I will consider its addition after ['sqlw'] option will be implemented.
Are you willing to write also exhaustive documentation after tis patch will be added into core class? I do not want to have undocumented features in phpMyEdit. They brings only problems. |
|
|
michal
2003-09-23 23:16
|
no problem with that,
will you let me know the way it should be written ?
i didn't find it in the documentation. |
|
|
hsicard
2003-10-22 23:57
|
Is it possible to get some more information on how to implement this?
Thanks. |
|
|
michal
2003-10-23 22:35
|
first you have to define your output function:
function my($value) { if($value < 0) return '<b>'.$value.'</b>';
return $value; }
or
function my($value) { return '<img src="'.$value.'">'; }
or any function of your need.
possible arguments of this function can be seen within the output-control.diff file, in function output_functionDisplay.
and then point your table definition to this function:
'output_function' => 'my($value)',
your function, called my, will be eval-ed within the pme code and the return-ed value will be displayed. |
|
|
michal
2003-11-22 14:44
|
the output function option was not available in view mode. latter diff solves this. |
|
|
michal
2003-12-15 01:15
|
i uploaded a nicer version of my patch. some duplicate code was removed, the Undisplayed option now works as expected, output functions can be referenced in LF and V mode, ...
now all i have to do i write some documentation ...
another feature is ability to reference to other columns in URL, via $row[col_name] associative array.
U (undisplayed) option can be used to be have reference to fields, which are not displayed. |
|
|
nepto
2004-01-12 07:47
|
Looking at the patch I see, eval() is used twice. Several CVS versions before I get rid of all the eval()s in the code. Thus it would be nice to also get rid of eval()s in the ['output_function'] feature.
I have idea how and will describe it sometimes when I find time between school exams, work and so on. ;-) |
|
|
hbernard
2004-01-12 09:39
|
also, it would be nice for consistency to have this named 'php' option
(maybe allowing 'phpw' in the future).
and more :
maybe let the 'virtual' thing ('U' options) away for now, and open after commit another bug report which will allow virtual for both 'php' and 'sql' field. |
|
|
michal
2004-01-12 11:21
|
i can make a patch against current CVS using substitueVars function.
and naming issue: 'php' - a nice name. how would you name function that displays header (like 'first and last name' with 'first' and 'last' clickable and searchable/filterable as if in separate columns ? )(i had a patch working, but it used weired class stuff ) phph ?
U: the patch will be maybe 10 lines :) |
|
|
hbernard
2004-01-12 11:29
|
1. patch : go on, 5.4 is waiting for you ;-)
2. 'php' : Nepto, what do you think ?
3. phph : I don't understand, sorry.
4. 'U' : I would prefer having a 'virtual' => true field's option (mainly because 'options' stuff is too much bloated, with 'ACPDLFV' on one side, and 'RHW' on the other side, which are completly different purpose...
I know patch is easy, this was in my first 'sqlw' patch 8-)) |
|
|
michal
2004-01-12 11:42
|
1. i will do it in a couple of days. are there any commits with cellDisplay function comming ?
3. will post an example
4. U says: 'do with it as if normal, but, at the last moment, don't display it' while 'V' says 'do with it as if normal, and display it'.
and if you have 'UVPD', it will be displayed in VPD and undisplayed/virtual in LF. while with virtual you would have to have 'VPD' plus 'virtual|LF' : more typing :) and conflicts if 'V' and 'virtual|V'. |
|
|
hbernard
2004-01-12 11:56
|
4. oh, sorry, I see we aren't calling 'virtual' the same thing.
As I understand you, 'U' fields are _real_ database field ?
But you need to access to these while you don't want to display ?
Then, shouldn't be the 'H' options patched to be just like it must be (requested but not displayed ?)
Or am I totally wrong about that ? |
|
|
michal
2004-01-12 12:04
|
U fields are standard fields defined via $opts['fdd']... (maybe with SQL option, values,...). i need access to them through $row array in output_function/php and URL options. ('use value from another field' bug).
AFAIK H is _always_ hidden (?) while U can be sometimes shown and sometimes hidden (UVPD). |
|
|
michal
2004-01-13 10:13
|
do you agree to make U/H option behave as described 4 posts above ?
then all we have to agree on is whether it's called U or H.
but let's finish PHP stuff (i like that name). do you agree with output_control-final.diff ? (with added new stuff from cellDisplay() into create_value() ). the patch just divides cellDisplay function into several ones. |
|
|
hbernard
2004-01-13 15:00
|
[NB I deleted previous bugnote, I was too tired, please forget]
Well, after browsing closer in your patch, here are some thoughts :
1. About 'virtual' semantic : what I called a virtual field is a field that don't exist in the table model (at the database level). U is not virtual, it is a way to have some informations available but not visible, no ?
2. About API and the U things :
(Sorry, I use this already in some scripts, but I should have post here this sample to explain better my idea :-/)
'options|ACLF' = 'ACLFH'
'options|VPD' = 'VPD'
is valid, and this should mean
"Hide in ACLF mode but let it available
and display in VPD"
then, this is equivalent to
'options' = 'UVPD'
Right ?
If you agree, there is no need (from an API point of view) to add another 'U' flag... We just have to be sure that this correctly handled in pme.
3. About implementation details :
I'm a bit worried by having new loops and extra processing in the "new" cellDisplay and create_row functions.
I was comparing old behaviour and new one, and, well, thinking that it will be processed for every row in list mode does not make very happy, but I may be wrong.
On the other side, I didn't profile the code, so maybe it costs quite nothing.
Another concern is having stored in _class_ the $row_* results.
But this is certainly to avoid multiple argument in the user defined output func. I need to think a bit more about it.
Well, what do you think ? I want to have this feature, don't doubt about that.
Thanks for your work and upcoming comments. |
|
|
michal
2004-01-13 15:53
|
1. yes, that is why i started to call it Undisplayed.
2. the question is wheather to make old (H) complex option usable by studying what it really does, or start a new (U) option for a single purpose. i would vote for second option. current mysterios behavior of H (mainly for AC) and desired behavior of U (for LFVPD).
another reason is mulitple user systems with 'options' defined for each user individually (in a db or somewhere).
3. new behaviour just splits cellDisplay and creates each row just once, at the beginning. (expect for rough_row, which is duplicate 7 rows).
storing rows: this is neccessary if we want to move towards free-form design. |
|
|
hbernard
2004-01-13 17:25
|
1. issue resolved.
2. After some investigations, H behavior is broken.
options = '' is the same as options = 'H...'
The syntax (already posted above) is certainly strange,
'options|ACLF' = 'ACLFH'
'options|VPD' = 'VPD'
but if find it not harder nor easier to read than the one with U.
(Certainly my point of view, not yours)
About your concern about fixing from a database, well, I doubt you can't adapt you model to this. Why not storing Undisplayed (ACLF) and displayed (VPD). just a field to add ;-)
The question i see :
- either make the 'H' option working like above
- either use your 'U' API
- either use another API style, which could be :
+use lowcase for allowing value without displaying :
'options' => 'aclfVPD' is the same as above.
+ use 'undisplayed' = 'ACLF' and 'options' = VPD (maybe the evident one)
+ some others ideas....
BAck to the maintainer to give us some thoughts..... (ohe, nepto, where are you ;-)
3. Storing row : Why not, but this is not really how pme usually handle those things... (class var are fixed var, which don't change during the script execution).
4. About the spliting of cellDisplay : Maybe I will make some profiling, If I have some time. But you're are certainly right. Can't we imagine the creation of row_rough and row_valus in the same loop ? |
|
|
michal
2004-01-13 20:56
|
i uploaded a diff against current v1.115. still with eval (just one).
API is as in previous examples, but 'output_function' was renamed to 'php', and row_rough is rough_row, values_row is untrimmed, new trimmed_row is trimmed, row_output is php_row. however these don't work due to bug 160.
4. yes, it's in the same loop. now just to make sure rough_row is correct, mainly with joining, masks...
3. i can't imagine without storing. in VPD mode, create_rows function is called in completely different place than cellDisplay (in a different function).
2. i vote for 'options' => 'AClfVPD' . ( least characters :-) ) |
|
|
hbernard
2004-01-13 22:31
|
2. Then maybe we agree on API ;-)
Using lowercase for undisplayed but available field is OK for me.
3/4/1 Reading Nepto comment on #160, some thoughts :
- weird url stuff can be as with 'sql' sample in forum
escape = false
php = myurlspecificfunc()
(in the func, you may access to all row values and build whatever hyperlink you want, no ?)
- So, maybe this feature could be more quickly in cvs, using (2) API if nepto agree with it, but without complex url string remplacement stuff. (For which I agree with performance concern).
Again thanks for this thread. If you agree with above, maybe a patch to close this issue ;-)))))))))))))))))) |
|
|
hbernard
2004-01-13 23:06
|
And also, I think your last eval can be avoided if you know about this sample :
<?php
function myfunc($myvar) {
return "<< ".$myvar." >>";
}
$nameofmyfunc = "myfunc";
echo $nameofmyfunc("Hello World");
?>
php is great, isn't it ? ;-)
And more :
- Really, don't create too much redondant array in the object (I saw you created the escape_row one......
- if you want to have access to all row value, maybe have the function prototype for 'php' option like
function myphpfunc($value, $pme_this);
which will allow 'basic' use and advanced use (yours ?) by allowing to read all the row values.
Sorry for too much post tonight. Will be quiet for a while, I promise. |
|
|
michal
2004-01-13 23:09
|
ok.
leave url stuff out.
p >< P will continue to separate bug report
can you confirm functionality of this patch ? |
|
|
michal
2004-01-13 23:21
|
eval: removing it will be a two line patch, while this whole one is pure rewriting of 184 lines. i just want to finish this. eval can be removed later.
pme_row - i don't use this one
rough_row - that's the one i need (datetime without mask and original value with 'values')
values_row - was divided into values_row and trimmed_row due to 'url' link_val.
php_row - that's the final one
escape_row - escaping is set while creating values_row. this one could be removed. |
|
|
hbernard
2004-01-14 22:28
|
well, really, I can't just say more.
It's up to Nepto to say where we should go.
It seems we agree on :
- separate 'U' API discussion to another bug
- separate url stuff and heavy $var substitution to another bug (maybe the one already open)
- API should be 'php'.
We disagree on :
- function prototype for output_function. :
I think we should have one, you seemed to think this not important, or this should be free syntax. But maybe I misunderstood you. The way you think to resolve the eval() issue has some consequences on this.
- I think the less we store in the class, the better it is.
You want to have access to multiple state of the row anywhere.
For my part, I can manually escape/trim the value when I need it.
I'm not saying you're wrong, just that we disagree.
Back to the maintainer. |
|
|
michal
2004-01-15 14:14
|
agreement: agreement
disagreement:
1. i don't understand. i think it's a discussion on how phpDisplay function should look. maybe make it 'return $value;' and open a separate bug ? :-)
2. i have a patch at home which removes escape_row. pme_row can be also removed. this would leave only 4 rows (rough, values, trimmed, php). passing this variables within LF can be made easily. PDV will be harder (two functions will have to changed, so not that hard) |
|
|
nepto
2004-01-16 12:27
|
Firstawal thank you guys for your wonderful work and effort. Sorry about my silence, I was very busy. Next time in the future, if you will need my opinion on something, do not hesitate to remind me via reminder (does it work here? ;-) private message or e-mail. I will immidiatelly try to find some time for writting few notes concerning to the topic.
Now notes to the topic:
[1] I agree with ['php'], ['phpw'] and eventually also with ['phph']. These names are very good idea.
[2] HBernard correctly pointed on one very important thing, what I have realized for couple of months. That is 'ACPDLFV' are ['options'] and 'RHW' are also options, but with completelly different purpose!
When thinking more abstractly about the topic, you will find there is even no way how to define HTML input element on Add/Copy/Change page. Yes, there is ['select'], however it has to be used only for filter page (as filter selection inputs).
Thus it could be really good idea to introduce new ['input'] option, what will:
* allow user to define what HTML input element should be used on Add/Copy/Change page
* cover 'RHW' options
If we want to be precious, we could add 'RHW' functionality also to ['select']. I can imagine selection based upon password input field, however it is strange. Just to be complex.
This will definitelly break BC. And also this should be considered in another bug report.
[3] I'm affraid that I do not understand the purpose of the 'U' option. However if it can be covered by other field option(s) with just using options variability (for example mentioned 'options|ACLF'=>'ACLFH' and 'options|VPD'=>'VPD'), we have to use already implemented way and not introduce new unneccessary options.
Of course if 'H' is not working properly, it has to be fixed. The question is, how exacly could behave properly working 'H' flag. The answer is quite simple: 'H' flag's aim is to put down HTML hidden input instead text/select/multiple input. This is its whole straightforward definition.
[4] If I correctly understand where part of the problem lies, we need something, what will work like 'H' (in meaning use DB column, but do not display it), but without writting HTML hidden input. 'H' could be used for this, but it could be dangerous, for example when it contains some kind of protected or sensitive content.
I'm not going to write more about this, since I'm not sure if I correctly understand an issue. Maybe I'm wrong. Let me know about this.
[5] Request for consideration. What do you think about making ['php'] option more simple:
'php' => 'some_function_name';
some_function_name() function should have certain concrete input parameters and should return value of the field. Prototypes for this function will be well-written in the documentation and now we definitelly avoid problems aroud eval() and substituteVars() stuff. Just a thought.
[6] This topic is rather more difficult, than I thought. Thus I'm considering to release 5.4 now, without "output_function" feature. The code in CVS is in quite good condition. The rewards for this are:
* cleaner tracking system
* fresh impuls to PME community
* checkpoint before implementing some heavy changes such as new ['input'], ['php'] etc.
If there is nothing special, I would like to roll out 5.4 and come back to discussion about this complex feature. |
|
|
michal
2004-01-16 13:27
|
i uploaded v5 of output_control. minor changes are: removal of escape_row, removal of rows in urlDisplay, removal of pme_row, some cleanup
what's left : basically nothing :), old cellDisplay function is renamed to create_value and is executed for each row in global, at the beggining. this makes it possible for 'php' option to work right (with $row array). if needed phpDisplay can be changed to
function phpDisplay ($k) /* {{{ */
{
// $k number of column
$row = $this->trimmed_row; // row as will be displayed
$name = $this->fds[$k]; // name of the column
$value = $row[$name];
return $value;
} /* }}} */
which will return $value (as if not called) and open a separate bug 'function phpDisplay'.
i know, commiting this patch has zero enhancement, but it is needed for 'php'. also, this patch is just manuall rewrtiting of code, so that is the reason i want it to finish as soon as possible. |
|
|
hbernard
2004-01-16 13:48
|
[1] 'php' API : fine, but I don't understand 'phph'... Maybe another bug report about this ?
[2][3][4] U/H/input weird API: OK, let's punt this post 5.4 and in another bug report.
[5] Prototype func : Yes, that is exactly what I was thinking about.
[6] Well, without 'URL', U/H stuff, maybe 'php' would be useless (there are workaround with 'sql').
So maybe you're right to punt overall feature.
However, if you agree with michal's way of rewriting of code, then you could consider to commit without 'php' API.
This will not introduce any new feature but allow to have shorter patch proposal in the future. |
|
|
hsicard
2004-02-11 15:53
|
I'm trying to apply the patch against phpMyEdit.class.-current-1.115.php using command patch in cygwin:
------------
patch -p0 --dry-run < output-control5.diff
------------
but I get the following error
------------
Hunk #1 succeeded...
Hunk #2 succeeded...
Hunk #3 succeeded...
**** malformed patch at line 42: @@ -1179,10 +1191,93
------------
I'm totally inexperienced applying patches and using CVS. I guess I'm making something wrong... Any hint? |
|
|
michal
2004-02-11 22:59
|
i'm sorry, i tried to manually remove some unimportant rows from the diff. it seems to confuse machines but not humans :)
a full class is uploaded |
|
|
michal
2004-02-15 22:26
|
i uploaded a new diff. some minor changes are:
function cellDisplay has now just one row
function create_rows now includes all stages of application of options (also with php and URL)
just one variable is added to the class ($this->row) which is the final row to be displayed.
else is left as in previous version.
i am sorry if automatic patch will fail, i really need to start organiziing my versions... |
|
|
hsicard
2004-02-19 17:34
|
Diff n°5 mess up variables like "$key" in URL definition.
I guess you solved it but in your last diff, not only machines but also humans get confused :)
It seems that function href_hidden_vars() is not defined neither in diff file nor in v1.115... Must be virtual!
Could you upload the full class? Thanks. |
|
|
michal
2004-02-19 23:17
|
http://platon.sk/forum/projects/viewtopic.php?t=249 (features forum, post data preservation) is where href_hidden_vars is defined. manual removal of unnecessary code is really dangerous :)
a full class is uploaded. the onle other experimental things are that href_hidden_vars(), bug 160 (substitute vars), bug 134 (join with filters)(partial fix). all of these are working. 134 can be removed harmlessly, removing 160 will loose functionality.
enjoy and comment whether working and what to improve.
free >< fixed design of 'php' functions
after looking at applications i have, i really use $value (60%), rest 40% is a mixture of $name, $rough_row and $row (==$trimmed_row)
however i will experiment on a mixture: free design split with "(" and then with "," and apply as mentioned fixed desigh (i had a working sample, but that was a long time ago)(but that should be a separate bug report) |
|
|
hsicard
2004-02-23 19:34
|
I'm trying the last uploaded class but I don't succeed in retrieving row's id (=$key) in php or URL features. $name and $page are OK but what I need is the "key" value.
It seems that variable $key is NULL in urlDisplay()... so your substituteVars() function may be inocent. Stuff like 'URL'=>'upload_doc.php?ref_doc=$key' or 'php'=>'display_img($key)' don't work for me.
Php feature would be so great though... Can it be a virtual field?
(see trick of bug 164. It actually works, 'sql' allows creation of virtual fields in List and View mode) |
|
|
michal
2004-02-23 19:38
|
as a workaround use $row[my_key_field] .
i will look at what's going wrong. |
|
|
michal
2004-02-23 19:42
|
well it wasn't that hard. change $key definition to
$key = $rough_row[$this->key];
in urlDisplay function.
stupid typo :) |
|
|
hsicard
2004-02-23 20:16
|
Even after new definition:
Retrieving url variable shows "rec=" only and trying to retrieve $key shows "$key".
Workaround $row[my_key_field] gives me: "$row[my_key_field]".
Using standard 5.4 on same form provides me right $key values in 'URL'... Are you sure of the uploaded class? |
|
|
michal
2004-02-23 22:10
|
i hope you changed $row[my_key_field] to $row[id] or what's relevant in your case (my_key_field should be the name of the field)
browsing through the uploaded file it looks the same, but i will upload new version. |
|
|
hsicard
2004-02-23 23:12
|
This URLdisp definition:
****
'URLdisp'=>'key=$key & name=$name & page=$page & row[iddocument]=$row[iddocument] & rough_row[iddocument]=$rough_row[iddocument]',
****
display with your code (n°7):
****
key=$key & name=tech_upload & page=document.php & row[iddocument]=$row[iddocument] & rough_row[iddocument]=$rough_row[iddocument]
****
while with 5.4 standard code (of course it does not interprete arrays):
****
key=3 & name=tech_upload & page=document.php & row[iddocument]=$row[iddocument] & rough_row[iddocument]=$rough_row[iddocument]
****
but at least $key is ok. :-(...
I'll try to get deeper into it to find out what's going on. Thanks for your help and support. |
|
|
michal
2004-02-24 10:21
|
is $value and $row[$name] working ?
if yes, it's probably substituteVars error, if no, it's somewhere in urlDisplay or create_rows.
($row["iddocument"] won't work while $row[iddocument] should) |
|
|
hsicard
2004-02-24 11:31
|
I got it: in your code, $key can only be used if displayed... In create_rows, $val, $rough_row, $values_row and trimmed_row are defined only if $k field is displayed. All works fine when I display the id.
I got rid of "if($this->displayed[$k])" in line 1406, so now it works fine even if $key field is not displayed...
Can I leave it like that or is it risky to fill arrays $val, $rough_row, $values_row with undisplayed fields? |
|
|
michal
2004-02-24 11:41
|
there is/will be a separate bug report on udisplayed fields (fields processed by create_rows but not displayed, eg available in $row array)
thanks for reporting current state, i will make a patch to make $key always available in $row array. (backward compatibility).
but it's quite safe to comment that if out. the arrays will be bigger but nothing bad will happen. |
|
|
michal
2004-02-24 13:40
|
uploaded new class should:
- make key field available in $row arrays at all cases
- 'lfV' syntax should display field in V mode, and make it in $row arrays in list and filter modes (discussed here on 13 january)
changes: displayed function (make case sensitive), almost_displayed (case insesitive version of displayed), 4 chagnes from displayed to almost_displayed in core class (3 in create_rows and 1 where creating sql query)
OT: can you jump between list and filter mode ? to make sure hidden_vars functions work. |
|
|
hsicard
2004-02-24 18:14
|
$key now ok in 'URL' feature. In 'php' feature it is not directly available through $key but through $row[id]. I already added some great new functionalities in my application thanks to 'php'. :)
I don't really know what you wanted me to test but "jumps" between list and filter mode worked fine for me. |
|
|
hbernard
2004-02-24 18:17
|
He wanted you to test :
List mode -> Filter mode -> make a filter -> List mode -> Filter mode
Did you keep your filter set ? |
|
|
hsicard
2004-02-24 18:41
|
Oui!
Filter set -> List Mode -> Back to Filter mode : filter set
Filter kept set with what I specified when I edited it again. |
|
|
hsicard
2004-02-24 20:27
|
Adding works, but changing don't actually change and deleting display following msg error:
***
Warning: array_keys(): The first argument should be an array in /... /phpMyEdit.class.php on line 2642
***
It seems that $this->rec value is lost in do_change_record() and do_delete_record() |
|
|
michal
2004-02-24 21:49
|
that's in connection with hidden_vars function
change at line 2409 (display_Record) to
echo $this->input_hidden_vars($this->rec);
(extra $this->rec)
rec value was not redefined in view mode |
|
|
hsicard
2004-02-25 14:14
|
Up to now it works OK with this last diff. To get 'php' virtual fields in list and view mode, I added 'sql'->'id'. Having some stuff in 'sql' allows the virtual field to be displayed, and 'php' outcome overwrite 'sql' value. |
|
|
michal
2004-02-29 23:52
|
new file uploaded, very few very minor changes. |
|
|
hsicard
2004-03-06 13:50
|
Do you know when 'php' feature is likely to be integrated in CVS version? |
|
|
nepto
2004-03-28 16:18
|
OK, many questions why this is stalling...
Code of the substituteVars() in uploaded form is very ugly. I cannot agree with that. Why we finally did not agree to function prototype? See my note from 2004-01-16 12:27.
I think that implementation in this way could be:
* cleaner (code)
* faster
* safer (in terms of security) |
|
|
hbernard
2004-03-29 17:17
|
prototype function : yes, that's what I said in my note from 2004-01-13.
substitute var has been discussed in another bug report. (and should stay there). |
|
|
michal
2004-03-29 23:11
|
at first i thought function prototype means my_func($value, $row, ...) (with many and changing arguments). but if it is like my_func($arguments) then it's ok for me. it will loose some functionality, but not much.
so example would be:
function my_func($arguments) { return $arguments['value'];}
$opts['fdd']['my_col']['php'] = 'my_func';
and (instead of eval) return $php_name($arguments); ($php_name and $arguments defined earlier).
can we implement it in this way ? |
|
|
michal
2004-04-09 16:34
|
longly awaited php option with fixed design patch is here. it is against current cvs 130.
features are as before, declaration is now with fixed design:
'php' => 'my_function'
and default is:
function my_function($arguments) {
return $arguments['value'];
}
where $arguments is associative array consisting of: name, value, key as value of key_rec, row and rough_row associative arrays.
a single value is to be returned. |
|
|
hbernard
2004-04-18 13:40
|
Uploaded a very simple patch (13 line added) for having a basic php option.
1. Some notes before :
I reread the whole thread before posting this.
I'm not saying this is a "full-featured" patch like those from michal.
I think that if nepto has not commit michal's patches into the main class, this can maybe be because patches are too big.
2. What the patch does :
* add a col_has_php func
* in display_*_record and in list_table funcs :
=> replace $row['qf'$k] with output of 'php' func (if defined)
* in set_values, apply 'php' if required
3. What it does not :
* implementation of phpw.=> should go in another bug report.
* playing with the 'virtual' thing. 'AcpDv' stuff is more general than 'php'.
4. What API is implemented :
- 'php' => 'name_of_my_func'
- the custom func take the (eventually joined) value of the field (qf...) and return another value.
5. Which issues this patch raises ?
* *when* should 'php' be applied ? I do it just after sql retrieving.
* the full $row is not currently available, but this is a two line patch ($this->row = $row at two different place).
Again, I know this doesn't make an overall rewrite of way PME hanlde all of the values stuff, but here is my proposal for having it simply/quickly in main class. |
|
|
michal
2004-04-18 22:01
|
well, the first thing i needed in 'php' was the 'row' array :-). to have this array is most of the patch. 'php' and 'AClfV' api is just two lines each (really)
5.1. i have it right before url (so value with datemasks, ... is available in $row array and value from the database is in $rough_row array).
5.2 $row is problem. i should have separated this into a separate bug, but that would produce a patch without solving anything or without any new feature.
i agree it's a quick hack that removes (in my needs great) part of the functionality (and most of the diff size :-) ) |
|
|
hbernard
2004-04-18 22:54
|
a) As said, I'm only trying to have basic 'php' in minimum change, because we need to go step by step if we want something.
Once this is the class, we can go with other bug report about other issues.
b) if we have $this->sql_row (I prefer this to rough row) defined before calling 'php' custom func, would you consider <'basic'> 'php' feature as implemented ? |
|
|
michal
2004-04-18 23:37
|
b. sql_row - good name
c. i believe that both of us want sql_row to be array('col_name' => 'its value') and not like array('qf4' => 'its value'). that's not that easy to achieve :)
d. let's make call to 'php' like
$this->fdd[$k]['php']($subst_ar);
where $subst_ar = array('name' => $this->fds[$k], 'value' => $value, ...);
which will allow change in future. so fixed design.
e. if you look at my patch, it's huge but fairly easy (just moves bunch of code around). it changes following
1. creates function almost_displayed (returns true for those that are displayed() and for $this->key)(and allow flV api as extra +2 lines)
2. changes displayed() to case sensitive (allow flV api)(extra)
3. modify cellDisplay to create_value() so that it returns both 'sql' and 'normal' values
4. extra function create_rows() which processes new create_row (2), then 'php', then 'url'.
5. function form 4 is called two times from core class
6. make cellDisplay just echo already ready $value
looking at the necessary changes to have proper sql_row (as in c.), it's really 10 extra lines and we have my patch :)
e. why 'php' in set_values ? for drop_down box, some possible outputs are bad (<b>value</b>), for normal view, it can be emulated with normal calls (with $value already changed by according to values)
most of all i am worried that patch to have $row available would take even longer than this bug. :) |
|
|
hbernard
2004-04-19 07:51
|
c.
$this->sql_row['qf'.$this->fdn['col_name']] isn't good enough ? ;-)
d. then, pass only $k. if you want to have 'php' a workaround for custom layout, I will disagree !!!
e/ we really have to separate issues.
f. Maybe we don't talk about the same 'php' feature ?
I can't imagine that an API will display thing in a way in list mode and in an other way in Dropdown or Filter set. Yeah, sql does it, but there are workarounds.... Hmmm, need to think about this.
g. Again, this hasn't been discussed. I can't see why you need *whole* $this->row ? couldn't it be the reverse. You got sql_row and for a given field you call a 'make_row_display' func in your custom func ? |
|
|
michal
2004-04-19 10:32
|
c. well, no. sometimes there is extra _idx, sometimes _timestamp, ...
i want sql_row to be as is in database, so without any 'values' options applied (even without joining, that's the _idx part)
d. if we want to have the same variable available in 'php' as in 'url' (seven pieces), how pass it to the 'php' function ? (note that the number might change in the future). we throw away 'my_func($value,$name)' syntax, and we want this feature to be BC when adding new variables.
e. we can fork this into three bugs:
- create row (which is to be displayed) in one place. (this will make extensions like export to CSV a lot easier, will allow to have $row available) (most of the patch)
- make 'php' api (and agree on api) (10 lines patch)
- make 'Vfl' api (3 lines patch)(last)
f. my intention of 'php' is to allow custom changes in displayed values on top of excelent set of pme features (not as an alternative), with possibility to access other than current fields values.
i've never intended nor needed to hack set_values(), i've used name create_value() for a completely different purpouse (at that time i didn't know set_values() exists :) ) |
|
|
hbernard
2004-04-22 15:59
|
This is a won't fix bug.
It has been splitted in
#238 : pure 'php' feature
#239 : availability of arrays of values to make 'php' more powerful
About what has been called "Undisplayed" or "Virtual" above, this will require another bug report, if this can not be handled by current API |
|
|
Login
Registration
Slovak
