Development
Threads by month
- ----- 2026 -----
- June
- May
- April
- March
- February
- January
- ----- 2025 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2024 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2023 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2022 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2021 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2020 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2019 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2018 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2017 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2016 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2015 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2014 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2013 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2012 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2011 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2010 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2009 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2008 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2007 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2006 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2005 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
March 2005
- 76 participants
- 522 discussions
Issue status update for http://drupal.org/node/11623
Project: Drupal
Version: cvs
Component: base system
Category: feature requests
-Priority: critical
+Priority: normal
Assigned to: pancho
Reported by: Goba
Updated by: Steven
Status: patch
- Localizing date formats seems like something that should be solved
properly in core once and tied into locale.module. We need a solution
which does not pose problems with mixing languages, and which does not
require us to keep adding more and more date formats to the list (e.g.
what about asian date formats? They don't have spaces.)
Your "if (!module_exist('locale_dateformat'))" solution is
unacceptable for core.
- What's the point of the 'tiny' date format if it's not used anywhere?
Where would it be used?
- Please don't post patches as zips, this is annoying. Just make one
global patch file instead of separate ones. Multiple files can be
posted with multiple follow ups.
- You need to see how this affects the date usage around Drupal, e.g.
the profile date field formatting, which depends on the date format
being in a specific format.
- Code style: check spaces around quotes. Also, avoid trailing spaces
on lines, this is untidy.
- This issue is not "critical" IMO. It is a new feature request, and as
such should probably wait until after 4.6, as 4.6 is in feature freeze.
- The apostrophes around the separator item seem a bit unnecessary.
Steven
Previous comments:
------------------------------------------------------------------------
October 15, 2004 - 19:52 : Goba
Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats.patch (1.99 KB)
Currently if I set a date format for the site, then it will be used with
all interface translations. Different languages however have different
needs when presenting dates. The Hungarian date format does not allow
the day name to be the first and it calls for a dot between the hour
and minute (yes, a dot not a colon). This means that Drupal ships with
no suitable date format for Hungarians. Also none of the Hungarian date
formats will be suitable on an English interface, while none of the
English date formats are acceptable on a Hungarian interface on a
bilingual site.
The only option is to make date formats translateable. Since these are
dynamic t() calls, extractor.php needs to be extended to include the
date formats shipped with drupal in the pot output, so translators will
get them (much like we do for some numbers and day names). I will take
care of extending extractor.php, once this patch gets applied.
------------------------------------------------------------------------
October 15, 2004 - 20:00 : Goba
Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats_0.patch (1.99 KB)
Well, the t() needs to be called on the date format set by the admin, so
I needed to fix three t() calls. Updated patch attached.
------------------------------------------------------------------------
October 15, 2004 - 20:01 : killes(a)www.drop.org
*g* I recall that there was a time when you didn't like this approach.
Anyway, I fully endorse it and would like to see it in 4.5. There is
also no suitable data format for German in core. And yes, that's a bug
not a feature request.
------------------------------------------------------------------------
October 15, 2004 - 20:05 : Goba
Yes, I didn't like it, because these are dynamic t()s. But since the
selected format always needs to be presented in the actual language, it
needs to be dynamic by nature. We have some of these dynamic t()s
already, handled specially in extractor.php.
------------------------------------------------------------------------
October 16, 2004 - 00:07 : Steven
I like this idea a lot, but I don't like the simplistic "wrap it all in
t" feature. There are instances where format_date output is intended
for script output. Changing those date formats would mean a bug in the
code or UI (for example the way a node date is specified and parsed).
Only those strings that are intended for user consumption should be
translated. That's why I think we need a more specialized date handling
component. It would deal with stuff like variations ('show hour/hide
hour'), long/short variations (we need to review the usage of
small/medium/long), as well as localizing them.
We would need to revise the formats shipped with Drupal, but this gets
tricky. For example would we only ship US-style AM/PM date formats with
core? Should we only have M/D/Y ordering, or also D/M/Y ordering? If we
move the time format settings to locale (where they do below, in
principle) we might get a drop in usability: moving from a simple
selection box, to requiring the whole localisation framework 'just to
switch from 12 hours to 24 hours'.
In some parts of the world, there is only one format for dates (e.g.
chinese/japanese 2004年10月16日 1時00分). For them, the only
variations are long vs short (whether to show the year, the time, etc),
in other parts of the world all bets are off and people use what they
like.
This would mean that if we provide a default set of formats, and let
them be translated through PO files, some languages might require more
items than provided and some languages would have to use duplicates to
fill all available options. On top of that, there is no guarantee that
format #3 in French matches format #3 in German, so you'd need to be
able to specify this per language.
Ideally, we would let a language define any number of date formats, and
let the user choose one of those per language. We could also provide a
custom option where you enter your own date format, for the realy picky
ones.
The question remains: what do we do for sites without locale enabled?
We can't go to requiring locale just to switch the date on a
non-localized site. In that case, perhaps we could do this:
- Have locale.module deal with date formats when it is enabled. Have a
per-language format setting, selected from a language-specific,
po-defined date format list.
- Have system.module implement a 'localization' admin menu item if
locale.module is /not/ enabled. This page carries the old selector,
which acts like a simplified version of locale's format selector, with
the list we current have in Drupal (i.e. variations of 24/12hours,
d/m/y, y/m/d, m/d/y, ...). To prevent confusion, we add a link to
system.module's version of the localization page, which instructs users
to enable locale.module for full localization abilities).
I know it sounds a bit weird (we certainly don't do this anywhere
else), but it would be consistent in terms of UI (the date format is
"localization" no matter how you look at it) and keep both
non-localized site admins and localized site admins happy, without
sacrificing too much ease of set up.
------------------------------------------------------------------------
October 16, 2004 - 10:34 : Goba
Steven, you say:
"/There are instances where format_date output is intended for script
output. Changing those date formats would mean a bug in the code or UI/
"
But the current approach is to translate the month and day names in
every case, so format_date **already** assumes that the date it outputs
is for human consumption and not for machines. So changing the order of
the fields in user date formats should not hurt. Sure it might not be a
useability dream, but it does not hurt.
By the way, adjusting date formats based on user language is only a
small piece of localisation not covered by Drupal core yet. I need to
use one mission statement and site name in all languages, and even my
primary and secondary links need to have the same text in all site
languages, since they are not translateable. If we are about to find a
generic solution for locale covered settings, we need to take these
into account too.
------------------------------------------------------------------------
October 29, 2004 - 01:10 : Steven
""so format_date *already* assumes that the date it outputs is for human
consumption and not for machines."
"
What I mean is changing the placement of the date components (day,
month, year). For numerical dates which are parsed by a program, this
will introduce bugs. I'd much rather have a cleaner date format
selection method integrated with locale.
------------------------------------------------------------------------
October 29, 2004 - 14:19 : Goba
Steven, the custom date formats are not translated, only those selected
by the admin. And the admin can select date types with spelled out
month/day names at any level, so they are not machine parsable anyway
in the current Drupal version. **This patch does not introduce bugs.**
I admit it does not look good, but there is no system for locale
dependant admin settings yet.
------------------------------------------------------------------------
October 29, 2004 - 16:05 : Steven
I'm saying we do need such a system.
With your patch, the format that the admin selects in language A will
not necessarily match the one in language B. We should go for
locale-defined formats with a locale-neutral list to fallback on.
------------------------------------------------------------------------
March 13, 2005 - 15:55 : pancho
Attachment: http://drupal.org/files/issues/dateformat.patch.zip (14.91 KB)
************************
DATEFORMAT-PATCH PACKAGE
************************
by pancho(a)suenderhauf.net
2005-03-13 03:07
This 'DateFormat' patch package only works with the latest CVS version
of Drupal 4.60 RC.
This 'DateFormat' patch package applies the following changes:
1.
It introduces a more flexible system of standard date and time formats
which helps both users and developers while staying
backwards-compatible. We often see special date formats be used in
modules, which is only second best, as these can neither
be adapted by the user nor localized. More flexibility allows the
developers to use the built-in format functions wherever
possible.
Changes in detail:
- The standard date formats used to be combined date&time-formats. This
is now split into separate date formats and time
formats, to allow dates and times to be rendered in a more flexible
way.
- Still it's backward compatible: the combined formats - while
deprecated - still exist.
- A "Tiny" format is added to the "Small", "Medium" and "Large"
formats.
2.
It enables Drupal to show e.g. German and Hungarian date formats (with
a colon as separator), which was _not_ possible
before.
3.
It prepares both the Drupal system and module developers for the
upcoming locale_dateformat module. This module will fully
localize date and time formats according to the localization of the
current user. The locale_dateformat module will be
available within the next weeks.
I'm always happy about suggestions and criticism, if constructive. You
can either write a comment on www.cvs.net or an email
to pancho(a)suenderhauf.net.
Bye, Pancho
-------------
INSTALLATION:
-------------
The package contains two patches for the latest CVS versions of Drupal
4.60 RC, both of which should be applied:
1. common.inc (patch for v.1.429)
2. system.module (patch for v.1.197)
There are no changes in database structure.
------------------------------------------------------------------------
March 13, 2005 - 15:59 : pancho
P.S. What the hell did I write about www.cvs.net? No, what I meant was
of course www.drupal.org (here), where you can write a comment!
------------------------------------------------------------------------
March 13, 2005 - 16:08 : pancho
I feel better if I refile the thread as a feature request. The only
thing you could describe as a bug was that German & Hungarian date
formats were not supported. The rest are definitely feature
improvements.
1
0
Issue status update for http://drupal.org/node/7582
Project: Drupal
Version: cvs
Component: node system
Category: bug reports
Priority: normal
Assigned to: killes(a)www.drop.org
Reported by: killes(a)www.drop.org
Updated by: killes(a)www.drop.org
Status: patch
Attachment: http://drupal.org/files/issues/revisions_23.patch (61.17 KB)
Thanks, Joshua, for catching this. node:title is there to stay.
killes(a)www.drop.org
Previous comments:
------------------------------------------------------------------------
May 5, 2004 - 17:25 : killes(a)www.drop.org
Currently all node revisions are stored in a serialized field in
node.table and retrieved for _each_ page view although they are rarely
needed. However, we have agreed that serializing data is bad and that
we should try to keep the memory foot print pf Drupal small.
Therefore I propose to create a separate revisions table which would be
in principle identical to the node table, only that it could have
several old copies of the same node. Extra data added by other modules
could be added in a serialized field unless we find a better solution.
------------------------------------------------------------------------
May 5, 2004 - 18:06 : jhriggs
I too think the serialized approach is less than desirable, but here's
an alternative. This would likely take some considerable rework in
core and contrib, but the following is how we handle similar types of
situations in our databases at work. It is more elegant that a
separate table, and avoids the (almost exact) duplication of a table.
Instead of separate tables, keep all revisions of nodes in the node
table as follows:
* add field: active (0/1 or Y/N)
* add field: revision
* every revision of a node is stored in the node table; however, only
one revision can be active at any given time
* nid can no longer be unique -- primary/unique key becomes (nid,
active)
* any time a node is loaded, updated (without revision), etc., the
active version is used.
Thoughts?
------------------------------------------------------------------------
May 5, 2004 - 18:57 : killes(a)www.drop.org
I am not opposed to your scheme, but I want to stress the following:
* Duplicating a table's structure is not bad (IMHO) as long as the
content is different.
* having two tables will allow us to have a rather small node table.
This is (maybe) a performance gain.
------------------------------------------------------------------------
May 5, 2004 - 19:37 : jhriggs
I don't necessarily think that duplicating a table's structure is _bad_.
It just seems to be wasteful and a pain to maintain. (Every change to
the node table is made twice...easy to do, but also easy to miss
perhaps.)
As for performance, as long as nid and the active indicator are
indexed, there shouldn't be any performance loss. Also, archiving an
old version when making a new revision will be much simpler: just
change the active indicator rather than copying an entire node to
another table (and ensuring everything gets copied...again a potential
maintainance issue).
To be honest, I would just like to see the serialized data go away,
regarless of what approach is taken.
------------------------------------------------------------------------
July 30, 2004 - 20:49 : Nick Nassar
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_07-30-2004.p… (10.47 KB)
I'm interested in using Drupal for a large scale wiki-type project. In
order to do this, I need revisions to be in their own table.
Attached is a patch to do just that. Most of the changes are pretty
self explanatory. Spreading out node data across two tables meant that
I had to add database functions to do locking/transactions. Without
this, race conditions in which the database becomes corrupted are
possible.
------------------------------------------------------------------------
July 30, 2004 - 20:54 : Nick Nassar
Oh yeah... The patch is a diff against Drupal CVS
------------------------------------------------------------------------
July 31, 2004 - 01:00 : Anonymous
Gerhard speaking.
Nick, thanks a lot for your nice patch! It saves me a great deal of
labour. I looked through it and immediately liked it. You not only put
the old revisions into a new table but also the current one. Do you
have an estimate how much more expensive the additional join is?
Besides a few minor coding style issues I found a major one: Just a few
hours before you uploaded your patch JonBob's node access patch hit
core. That means your patch won't apply anymore as all the queries you
change have been changed. Can I bug you to update your patch?
------------------------------------------------------------------------
July 31, 2004 - 02:11 : Anonymous
Also I think that your upgrade path loses existing revisions.
------------------------------------------------------------------------
July 31, 2004 - 03:39 : drumm
I think this is the proper way to do things. No columns are duplicated,
there is no serialized data, and only the fields that are logically
revised are stored. Nothing jumped out at me as a way to have my node
modules be able to keep a table of revisions of additional fields. I'm
guessing this could be done within the confines of _insert and _update.
Assuming the upgrade path works and modules can extend it I give it a
+1.
------------------------------------------------------------------------
July 31, 2004 - 15:40 : Nick Nassar
It figures that just as I finish a big patch, another patch comes along
and breaks it. Oh well, it should be a pretty easy to fix. I'll work on
it.
Fixing the upgrade path to keep revisions should be fairly painless.
I found another issue that needs to be fixed before this patch gets
merged. There format of a node needs to be stored for each revision.
Otherwise, for modules that store a format for the nodes, such as page
and book, if you write one revision in PHP and the next in HTML, the
PHP revision will be displayed as HTML. This is part of a larger issue
of how node modules should store revisions of additional fields. I
think each module that wants to do this should create another table
with (nid, revid) as the primary key. Just as when they want to add
fields to a node they create another table with nid as the primary key.
As far as performance goes, for sites that make heavy use of revisions,
an extra join on primary keys is going to be a lot faster than grabbing
all of the revisions from that database everytime. We would need to run
benchmarks to determine is the overall difference in speed is for an
average site is a gain or a loss. I'm guessing it's very minor either
way.
------------------------------------------------------------------------
August 23, 2004 - 14:55 : Nick Nassar
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_08-23-2004.p… (10.92 KB)
Here's an updated patch against CVS that puts revisions in their own
table, provides an upgrade path, and fixes the format related bugs in
the last patch.
Hopefully, this can make it into CVS as soon as the freeze is over.
------------------------------------------------------------------------
August 23, 2004 - 15:10 : moshe weitzman
Interesting patch ... drumm's question is still outstanding. how do
modules store revisions of their fields? Are they expected to manage
this on their own? Thats not how it works today.
As an aside, i am seeing profile_ fields in my node.revisions column.
One could argue that those need not be saved. They pertain to the node
author, not to the node itself.
------------------------------------------------------------------------
August 23, 2004 - 17:14 : Nick Nassar
Having modules be responsible for storing revisions of their own fields
is a side-effect of storing revision data in tables. There's really no
way around it. However, revisions generally don't make sense for node
types that don't have PHP/HTML content, such as polls. I think it's
going to be a pretty rare scenario for a new node type to want another
field to change per-revision, so it's a pretty good trade-off.
Storing fields that shouldn't be part of revisions, such as the
profile_ fields, is a side-effect of storing revisions as serialized
objects. Applying this patch will free up that wasted space. :)
------------------------------------------------------------------------
August 23, 2004 - 18:20 : Anonymous
There should be a hook that let's the module choose whether it supports
history. This way a module author can prevent the user from doing
something that may break his module or just cause undefined behavior.
If the module doesn't support history then don't let the user/admin
choose to add history to nodes of that type.
Craig
------------------------------------------------------------------------
August 23, 2004 - 20:23 : Nick Nassar
I agree, there should be an API change to make specifying support for
revisions easier. In the interests of keeping patches small and keeping
to one change per patch, I think the API change should be a separate
issue.
A sort of ad-hoc API to decide whether or not a module supports
revisions by default already exists. Instead of having a hook, modules
set the default value of the "Create new revision" field in the edit
form. The admin can change this option in
admin/node/configure/defaults. This patch doesn't change that.
Revisions are broken for node types that have their own database
structure, like polls, even when storing them as serialized objects.
This patch doesn't change that, either.
------------------------------------------------------------------------
October 26, 2004 - 03:35 : moshe weitzman
I'm guessing that someone is going to have to demonstrate that this
patch performs as well as current drupal before it gets comitted. i
think this patch is a few benchmarks from being comitted.
------------------------------------------------------------------------
October 27, 2004 - 02:04 : Nick Nassar
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004.p… (11 KB)
I ran some really unscientific benchmarks, and it looks like this patch
has a negligible affect on performance.
I used apache bench and the database from theregular.org, which doesn't
contain any revisions (worst case scenario for this patch) and contains
several hundred nodes. Both the patched and unpatched versions hovered
between 2.36 and 2.38 requests per second.
The command I used was:
ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2'
http://192.168.0.100/
An updated patch that should apply to CVS is attached.
------------------------------------------------------------------------
October 27, 2004 - 02:05 : Nick Nassar
I ran some really unscientific benchmarks, and it looks like this patch
has a negligible affect on performance.
I used apache bench and the database from theregular.org, which doesn't
contain any revisions (worst case scenario for this patch) and contains
several hundred nodes. Both the patched and unpatched versions hovered
between 2.36 and 2.38 requests per second.
The command I used was:
ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2'
http://192.168.0.100/
An updated patch that should apply to CVS is attached.
------------------------------------------------------------------------
October 27, 2004 - 02:05 : Nick Nassar
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004.p… (11 KB)
I ran some really unscientific benchmarks, and it looks like this patch
has a negligible affect on performance.
I used apache bench and the database from theregular.org, which doesn't
contain any revisions (worst case scenario for this patch) and contains
several hundred nodes. Both the patched and unpatched versions hovered
between 2.36 and 2.38 requests per second.
The command I used was:
ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2'
http://192.168.0.100/
An updated patch that should apply to CVS is attached.
------------------------------------------------------------------------
November 15, 2004 - 06:05 : elias1884
please overthink the revision system default workflow as well. don't
look at the revision system as an isolated system but as a part of the
whole workflow system!
if you combine revisions with the moderation queue the most logic
default workflow would be like that:
auth user creates node (revision #0)
admin approves the node (status = 1, moderation = 0)
=> node publicly available
auth user finds typo and changes node (revision #1, status = 0,
moderation = 1)
-------
what happens at that point at the moment is, that the node is not
accessible anymore at all until the new revision is approved by admin.
of course the new revision should not go online until reviewed and
approved, this is absolutely correct, but there is no reason to not
take the old revision offline, since it was already approved and should
therefore be online until the new revision is approved. it is not
practical if a node disappears only because the author corrected a
typo.
-------
admin approves the node (status = 1, moderation = 0)
eventhough I first thought a plain boolean active field would not be
capable of providing that functionality if finally came to the
conclusion, that it can. The only thing to do is to not set that bit,
when a new revision is created, but when it is approved (in case
moderation is activated under default workflow). Every revision should
have its own moderation, status and active field and on approval they
are set like this (status=1, moderation=0, active=Y).
When you wanna rollback to an old revision, you can chose between all
revisions that already have the moderation bit set back to 0 again and
the published to 1. There should be an extra permission for rollback!
another concern that I have about the default workflow is, that users
can't see the content, they have just created, when moderation is
enabled. Eventhough, there is a big fat "submission accepted" presented
after submissions, unexperienced users tend to question the information
those stupid tincans give them, if they can't find their content
afterwards. Many users are really lazy bastards and they don't even
read the status messages. The best feedback about whether his story was
submitted successfully or not of course is, if he can find the story
somewhere on the site, maybe with a status message on top of it,
mentioning, that the content is currently not publicly available since
it has not been approved yet. there should be a my content section
under my account, like somebody is trying to do with the workspace
module I guess.
so my suggestion is to make (status=0, moderation=1) still available
for the creator under a my content section somewhere!
------------------------------------------------------------------------
November 24, 2004 - 05:21 : Nick Nassar
I agree. The current workflow for moderation queues and revisions needs
to change, but this patch isn't the place for it. The patch is already
too big, and it only does the backend stuff.
Instead of adding more to this patch and making it take even longer to
get into core, would you mind creating a new issue for your UI
suggestions, so the those changes can be added as a separate patch?
Thanks,
Nick
------------------------------------------------------------------------
December 11, 2004 - 13:26 : Dries
This patch is _much_ needed so I'd love to see someone revive it. In
order for this patch to be accepted, the following needs to be done:
Update this patch to CVS HEAD.
Rename revid to vid.
Rename node_rev to node_revisions.
Rename node_rev.changed to node_revisions.timestamp.
Rename $rnode to $revision.
Fix the coding style to match Drupal's: proper spacing, single quotes
where possible, proper variable names.
Benchmark this patch with a large database with enough revisions. I'd
be happy to benchmark this on my local copy of the drupal.org database.
The book.log field should probably move to the node_revisions table.
This can be done in a separate patch.
Investigate whether transactions are well-supported.
------------------------------------------------------------------------
December 13, 2004 - 01:25 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004-r… (11.02 KB)
I've worked a bit on the patch (coding style issues as mentioned by
Dries). One thing I noticed is that the patch uses REPLACE. IIRC this
needs to be chagned to "UPDATE, if fail INSERT" for pgsql
compatibility.
Nick, are you still interested in working on that patch? I'd like to
know how it works on your site and work on getting it into core.
------------------------------------------------------------------------
December 13, 2004 - 13:33 : Dries
Gerhard: your patch does not apply.
------------------------------------------------------------------------
December 13, 2004 - 18:10 : killes(a)www.drop.org
Yes, I know, that was the same version as I mailed to you earlier.
------------------------------------------------------------------------
December 13, 2004 - 22:02 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions.patch (52.96 KB)
Ok, upüdated the patch to cvs.
------------------------------------------------------------------------
December 14, 2004 - 09:58 : Dries
Some more comments:
db_begin_transaction() and db_end_transaction() do not belong in
database.inc, but in database.mysql.inc and database.pgsql.inc
respectively.
The node module calls node_revisionsision_list() which is not defined.
(Fxed that on my local copy.)
Do db_begin_transaction() and db_end_transaction() deprecate Jeremy's
table locking patch?
The upgrade path assigns the wrong user ID to each revision.
The upgrade path assigns the wrong date to each revision (that or a
node's revision page shows the wrong usernames/dates).
The coding style needs a bit of work, but we can worry about that
later.
------------------------------------------------------------------------
December 14, 2004 - 18:34 : Nick Nassar
If you need any help getting those things fixed, just let me know.
------------------------------------------------------------------------
December 14, 2004 - 18:50 : Nick Nassar
How this relates to Jeremy's node locking patch:
There was lots of discussion, and node locking was decided against
because from an end user point of view you never want a node to be
locked. He's now advocating for a much simpler patch that warns users
if their changes will overwrite someone elses. That patch still has a
race condition, which might be fixed using db_begin_transaction().
http://drupal.org/node/6025
------------------------------------------------------------------------
December 14, 2004 - 23:26 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_0.patch (55.96 KB)
Here is an updated patch that tries to address Dries concerns.
------------------------------------------------------------------------
December 15, 2004 - 09:32 : Dries
Attachment: http://drupal.org/files/issues/revisions-bug.png (76.06 KB)
It didn't fix the aforementioned bugs. See attached screenshot.
------------------------------------------------------------------------
January 6, 2005 - 21:15 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_1.patch (51.77 KB)
Ok, here is a new version. Dries and myself worked hart at it, so please
have a look.
what is still missing
- database upgrades for the core modules with an own table
- contrib modules need an upgrade too.
- do we need nid and vid in both the node and the node_revisions table?
- the amount of sql queries means a good stress testing for large
databases.
------------------------------------------------------------------------
January 19, 2005 - 22:43 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_2.patch (49.49 KB)
Here is an updated patch. We discussed to keep the current title in node
module and also in the revisiosn table. This is content duplication but
will save many joins as many queries only need the title of a node.
Discussion is welcome.
------------------------------------------------------------------------
January 20, 2005 - 00:33 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_3.patch (29.93 KB)
I've implemented the aforementioned solution. This makes the patch much
smaller. The patch now also removes taxonomy_node_has_term() which
wasn't used anywhere. I'd really apprciate if some people could test
drive the patch. It will be another huge improvement for 4.6.
------------------------------------------------------------------------
January 20, 2005 - 01:05 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_4.patch (30 KB)
Another revision. Steven didn't like my literal $node->vid in queries.
------------------------------------------------------------------------
January 20, 2005 - 02:10 : killes(a)www.drop.org
- database upgrades for the core modules with an own table
- contrib modules need an upgrade too.
- do we need nid and vid in both the node and the node_revisions table?
- the amount of sql queries means a good stress testing for large
databases.
These issues are still open, btw. Especially the first one needs to be
tackled.
------------------------------------------------------------------------
January 25, 2005 - 21:11 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_5.patch (51.13 KB)
Here is a patch that has the database tables updated for forum, book,
and page module.
------------------------------------------------------------------------
January 29, 2005 - 23:55 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_6.patch (49.18 KB)
Yet another update to keep it working with head. The patch now also
removes the table definitons for the page table.
------------------------------------------------------------------------
January 29, 2005 - 23:57 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_7.patch (55.69 KB)
Sorry, that was the old version, this is the right one.
------------------------------------------------------------------------
January 31, 2005 - 20:55 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_8.patch (55.71 KB)
Updated once more.
------------------------------------------------------------------------
January 31, 2005 - 21:52 : Dries
Anyone to help review/test this?
------------------------------------------------------------------------
January 31, 2005 - 22:22 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_9.patch (49.29 KB)
Updated again, the update functions occurred twice. Thanks Bart.
------------------------------------------------------------------------
February 2, 2005 - 01:27 : killes(a)www.drop.org
Don't know if the db I am using is corrupted or what. I still do have
some didficulties.
The latest patch is attached.
------------------------------------------------------------------------
February 2, 2005 - 01:27 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_10.patch (49.67 KB)
I am probably slowly going mad ...
------------------------------------------------------------------------
February 2, 2005 - 02:54 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_11.patch (48.95 KB)
The update issue still needs investigating. This patch is updated for
cvs.
------------------------------------------------------------------------
February 2, 2005 - 21:20 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_12.patch (49.83 KB)
Ok, here is a new version. I've solved my troubles with book.module.
There are still some issues with forum module. Possibly due to
inconsistent database.
------------------------------------------------------------------------
February 2, 2005 - 22:31 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_13.patch (49.83 KB)
Turns out the drupal.org database had indeed some quirks. Please run
this query in your oldest db and tell me the result:
select nid,type from node where type like '%/%';
If you get a non-zero result we might need to add another security
update.
The patch could use still more testing, though.
------------------------------------------------------------------------
February 3, 2005 - 02:16 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_14.patch (49 KB)
Ok, we are getting somewhere. At a first glance the update is working.
There is a problem remaining: the revisions tab will be shown whether
the node has revisions or not. Not sure we can/need to fix this.
People with a drupal.org account can log in at
http://killes.drupaldevs.org/revision/ and poke around. Your
permissions will be the same as on drupal.org. Feel free to vreak
everything but don't forget to file complaints here. (Note: this is
only a pruned version of the drupal.org database with all project nodes
and nodes with nids > 7000 dropped).
------------------------------------------------------------------------
February 3, 2005 - 05:19 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_15.patch (52.39 KB)
There was some error in node_save and also the patches to the
database.inc files got lost...
------------------------------------------------------------------------
February 3, 2005 - 08:07 : robertDouglass
Submitting book pages doesn't work on your test site. It puts the entire
content of the preview inside the body textarea. I wrote a sentence in
the body and the log, and pressing preview put several lines of HTML
containing both sentences in the body textarea on the preview page,
plus the book page wouldn't submit.
-R
------------------------------------------------------------------------
February 3, 2005 - 08:50 : Junyor
0 results here. I started using Drupal with version 4.4, though.
------------------------------------------------------------------------
February 4, 2005 - 00:56 : killes(a)www.drop.org
@Junyor: Thanks, that's a good sign. Maybe somebody else has an older db
to try.
@robertDouglass: The first effect you describe is due to drupaldevs
running on PHP 5. I am unsure why the second thing does not work. In
node_save() the node object has a nid although there is none in the
form. Very strange.
I've enabled display of db queries on the testsite.
------------------------------------------------------------------------
February 4, 2005 - 20:17 : dmjossel
No results here on the query:
select nid,type from node where type like '%/%';
On a database that was put in place prior to Drupal 4 and is now
running on 4.5.2.
------------------------------------------------------------------------
February 4, 2005 - 21:44 : killes(a)www.drop.org
@dmjossel: thanks.
@all. The strange problem I reported was apparently php 5 related.
After applying Steven's php 5 patch it went away. One error is
remaining: If I create a new forum topic it will be shown as part of
the book on preview. Hmm, that was due to a db that got corrupted
during testing so that is fixed too.
Please poke around at the test site and look if you find more errors.
------------------------------------------------------------------------
February 5, 2005 - 08:16 : Steven
By the way, I just remembered that Drupal.org has some blogs lingering
on in the database even though blog.module is not enabled. Perhaps this
is causing troubles?
------------------------------------------------------------------------
February 5, 2005 - 12:22 : Anonymous
I can't see why it would. Drupal.org will need extra updates for images
and project nodes because those have their own tables. GK.
------------------------------------------------------------------------
February 6, 2005 - 13:49 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_16.patch (52.49 KB)
Updated to apply to cvs again.
------------------------------------------------------------------------
February 22, 2005 - 21:15 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_17.patch (49.64 KB)
Updated again.
All we need is a patch to upload module and an upgrade path for it.
------------------------------------------------------------------------
March 4, 2005 - 05:22 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_18.patch (52.31 KB)
Updated once more. Moved log field from book to node_revisions table as
discussed in Antwerp. upload module still missing.
We need to decide under which circumstances the log field should be
displayed. Should that be added to the workflow? Should it depend on
the revisions setting?
------------------------------------------------------------------------
March 5, 2005 - 20:27 : Anonymous
Attachment: http://drupal.org/files/issues/revisions_20.patch (75.52 KB)
Ok, here it is: Yet another revision of this grrrrreat patch.
Changes from previous versions:
- supports versioning for uploaded files. A problem is that if you
delete a file, it will be gone for all revisions.
- the log field is now in the node_revsions table, but each module has
to decide whether to show it or not.
I've implemented it for the page and the book type odes. Also, the
field can be edited when adding non-book nodes to the book. The log is
displayed on the revisions page and if a node is moderated.
- the revisions are moved to an old_revisions table to a) get the node
table smaller and b) still leave the mavailable for contrib modules
that want to retreive old version data.
The patch has been applied to killes.drupaldevs.org/revision where it
can be tested by anybody (especially people who have "site admin"
rights on drupal.org)
The database is from drupal.org and you should b able to log in with
your pass or simply mail yourself a new one.
Gerhard
------------------------------------------------------------------------
March 5, 2005 - 20:51 : Anonymous
Attachment: http://drupal.org/files/issues/revisions_21.patch (59.42 KB)
BTW, I marked this a bug because atm the revisions field can grow quite
big. Neil has reported problems from some users who were not able to
load some nodes due to to many large revisions.
Also, som unrelated stuff crept into the patch. New version attached.
------------------------------------------------------------------------
March 8, 2005 - 06:56 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_22.patch (60.29 KB)
Ok, I think I got it.
Changes to last version:
- uploads are no properly versioned.
Missing are still pgsql checks and updates.
------------------------------------------------------------------------
March 10, 2005 - 17:58 : Anonymous
Was able to get http://drupal.org/files/issues/revisions_21.patch to
work with drupal-cvs.tar.gz (10 March 2005) by:
- includes/database.mysql.inc: Commenting out duplicates for functions
function db_begin_transaction and function db_commit_transaction
- modules: node.module: Removing "'title' => $node->title," from
$node_table_values variable declaration and removing "'title' =>
"'%s'"," from "$node_table_types" variable declaration.
Happy to submit a patch if requested. I'll watch this thread.
------------------------------------------------------------------------
March 12, 2005 - 00:59 : killes(a)www.drop.org
The duplicate function has been removed in rev 22 of this patch.
Why do you think the changes in node_save are needed? Titles are saved
in both tables for performance reasons.
------------------------------------------------------------------------
March 13, 2005 - 17:12 : jlerner
Hi - I posted comment #62. The changes to node_save appear to be needed
because recent patches (both 21 and 22) remove the field 'title' from
table 'node'. So without the changes to node_save, node.module is
broken and generates errors.
Joshua
1
0
Issue status update for http://drupal.org/node/7582
Project: Drupal
Version: cvs
Component: node system
Category: bug reports
Priority: normal
Assigned to: killes(a)www.drop.org
Reported by: killes(a)www.drop.org
Updated by: jlerner
Status: patch
Hi - I posted comment #62. The changes to node_save appear to be needed
because recent patches (both 21 and 22) remove the field 'title' from
table 'node'. So without the changes to node_save, node.module is
broken and generates errors.
Joshua
jlerner
Previous comments:
------------------------------------------------------------------------
May 5, 2004 - 16:25 : killes(a)www.drop.org
Currently all node revisions are stored in a serialized field in
node.table and retrieved for _each_ page view although they are rarely
needed. However, we have agreed that serializing data is bad and that
we should try to keep the memory foot print pf Drupal small.
Therefore I propose to create a separate revisions table which would be
in principle identical to the node table, only that it could have
several old copies of the same node. Extra data added by other modules
could be added in a serialized field unless we find a better solution.
------------------------------------------------------------------------
May 5, 2004 - 17:06 : jhriggs
I too think the serialized approach is less than desirable, but here's
an alternative. This would likely take some considerable rework in
core and contrib, but the following is how we handle similar types of
situations in our databases at work. It is more elegant that a
separate table, and avoids the (almost exact) duplication of a table.
Instead of separate tables, keep all revisions of nodes in the node
table as follows:
* add field: active (0/1 or Y/N)
* add field: revision
* every revision of a node is stored in the node table; however, only
one revision can be active at any given time
* nid can no longer be unique -- primary/unique key becomes (nid,
active)
* any time a node is loaded, updated (without revision), etc., the
active version is used.
Thoughts?
------------------------------------------------------------------------
May 5, 2004 - 17:57 : killes(a)www.drop.org
I am not opposed to your scheme, but I want to stress the following:
* Duplicating a table's structure is not bad (IMHO) as long as the
content is different.
* having two tables will allow us to have a rather small node table.
This is (maybe) a performance gain.
------------------------------------------------------------------------
May 5, 2004 - 18:37 : jhriggs
I don't necessarily think that duplicating a table's structure is _bad_.
It just seems to be wasteful and a pain to maintain. (Every change to
the node table is made twice...easy to do, but also easy to miss
perhaps.)
As for performance, as long as nid and the active indicator are
indexed, there shouldn't be any performance loss. Also, archiving an
old version when making a new revision will be much simpler: just
change the active indicator rather than copying an entire node to
another table (and ensuring everything gets copied...again a potential
maintainance issue).
To be honest, I would just like to see the serialized data go away,
regarless of what approach is taken.
------------------------------------------------------------------------
July 30, 2004 - 19:49 : Nick Nassar
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_07-30-2004.p… (10.47 KB)
I'm interested in using Drupal for a large scale wiki-type project. In
order to do this, I need revisions to be in their own table.
Attached is a patch to do just that. Most of the changes are pretty
self explanatory. Spreading out node data across two tables meant that
I had to add database functions to do locking/transactions. Without
this, race conditions in which the database becomes corrupted are
possible.
------------------------------------------------------------------------
July 30, 2004 - 19:54 : Nick Nassar
Oh yeah... The patch is a diff against Drupal CVS
------------------------------------------------------------------------
July 31, 2004 - 00:00 : Anonymous
Gerhard speaking.
Nick, thanks a lot for your nice patch! It saves me a great deal of
labour. I looked through it and immediately liked it. You not only put
the old revisions into a new table but also the current one. Do you
have an estimate how much more expensive the additional join is?
Besides a few minor coding style issues I found a major one: Just a few
hours before you uploaded your patch JonBob's node access patch hit
core. That means your patch won't apply anymore as all the queries you
change have been changed. Can I bug you to update your patch?
------------------------------------------------------------------------
July 31, 2004 - 01:11 : Anonymous
Also I think that your upgrade path loses existing revisions.
------------------------------------------------------------------------
July 31, 2004 - 02:39 : drumm
I think this is the proper way to do things. No columns are duplicated,
there is no serialized data, and only the fields that are logically
revised are stored. Nothing jumped out at me as a way to have my node
modules be able to keep a table of revisions of additional fields. I'm
guessing this could be done within the confines of _insert and _update.
Assuming the upgrade path works and modules can extend it I give it a
+1.
------------------------------------------------------------------------
July 31, 2004 - 14:40 : Nick Nassar
It figures that just as I finish a big patch, another patch comes along
and breaks it. Oh well, it should be a pretty easy to fix. I'll work on
it.
Fixing the upgrade path to keep revisions should be fairly painless.
I found another issue that needs to be fixed before this patch gets
merged. There format of a node needs to be stored for each revision.
Otherwise, for modules that store a format for the nodes, such as page
and book, if you write one revision in PHP and the next in HTML, the
PHP revision will be displayed as HTML. This is part of a larger issue
of how node modules should store revisions of additional fields. I
think each module that wants to do this should create another table
with (nid, revid) as the primary key. Just as when they want to add
fields to a node they create another table with nid as the primary key.
As far as performance goes, for sites that make heavy use of revisions,
an extra join on primary keys is going to be a lot faster than grabbing
all of the revisions from that database everytime. We would need to run
benchmarks to determine is the overall difference in speed is for an
average site is a gain or a loss. I'm guessing it's very minor either
way.
------------------------------------------------------------------------
August 23, 2004 - 13:55 : Nick Nassar
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_08-23-2004.p… (10.92 KB)
Here's an updated patch against CVS that puts revisions in their own
table, provides an upgrade path, and fixes the format related bugs in
the last patch.
Hopefully, this can make it into CVS as soon as the freeze is over.
------------------------------------------------------------------------
August 23, 2004 - 14:10 : moshe weitzman
Interesting patch ... drumm's question is still outstanding. how do
modules store revisions of their fields? Are they expected to manage
this on their own? Thats not how it works today.
As an aside, i am seeing profile_ fields in my node.revisions column.
One could argue that those need not be saved. They pertain to the node
author, not to the node itself.
------------------------------------------------------------------------
August 23, 2004 - 16:14 : Nick Nassar
Having modules be responsible for storing revisions of their own fields
is a side-effect of storing revision data in tables. There's really no
way around it. However, revisions generally don't make sense for node
types that don't have PHP/HTML content, such as polls. I think it's
going to be a pretty rare scenario for a new node type to want another
field to change per-revision, so it's a pretty good trade-off.
Storing fields that shouldn't be part of revisions, such as the
profile_ fields, is a side-effect of storing revisions as serialized
objects. Applying this patch will free up that wasted space. :)
------------------------------------------------------------------------
August 23, 2004 - 17:20 : Anonymous
There should be a hook that let's the module choose whether it supports
history. This way a module author can prevent the user from doing
something that may break his module or just cause undefined behavior.
If the module doesn't support history then don't let the user/admin
choose to add history to nodes of that type.
Craig
------------------------------------------------------------------------
August 23, 2004 - 19:23 : Nick Nassar
I agree, there should be an API change to make specifying support for
revisions easier. In the interests of keeping patches small and keeping
to one change per patch, I think the API change should be a separate
issue.
A sort of ad-hoc API to decide whether or not a module supports
revisions by default already exists. Instead of having a hook, modules
set the default value of the "Create new revision" field in the edit
form. The admin can change this option in
admin/node/configure/defaults. This patch doesn't change that.
Revisions are broken for node types that have their own database
structure, like polls, even when storing them as serialized objects.
This patch doesn't change that, either.
------------------------------------------------------------------------
October 26, 2004 - 02:35 : moshe weitzman
I'm guessing that someone is going to have to demonstrate that this
patch performs as well as current drupal before it gets comitted. i
think this patch is a few benchmarks from being comitted.
------------------------------------------------------------------------
October 27, 2004 - 01:04 : Nick Nassar
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004.p… (11 KB)
I ran some really unscientific benchmarks, and it looks like this patch
has a negligible affect on performance.
I used apache bench and the database from theregular.org, which doesn't
contain any revisions (worst case scenario for this patch) and contains
several hundred nodes. Both the patched and unpatched versions hovered
between 2.36 and 2.38 requests per second.
The command I used was:
ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2'
http://192.168.0.100/
An updated patch that should apply to CVS is attached.
------------------------------------------------------------------------
October 27, 2004 - 01:05 : Nick Nassar
I ran some really unscientific benchmarks, and it looks like this patch
has a negligible affect on performance.
I used apache bench and the database from theregular.org, which doesn't
contain any revisions (worst case scenario for this patch) and contains
several hundred nodes. Both the patched and unpatched versions hovered
between 2.36 and 2.38 requests per second.
The command I used was:
ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2'
http://192.168.0.100/
An updated patch that should apply to CVS is attached.
------------------------------------------------------------------------
October 27, 2004 - 01:05 : Nick Nassar
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004.p… (11 KB)
I ran some really unscientific benchmarks, and it looks like this patch
has a negligible affect on performance.
I used apache bench and the database from theregular.org, which doesn't
contain any revisions (worst case scenario for this patch) and contains
several hundred nodes. Both the patched and unpatched versions hovered
between 2.36 and 2.38 requests per second.
The command I used was:
ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2'
http://192.168.0.100/
An updated patch that should apply to CVS is attached.
------------------------------------------------------------------------
November 15, 2004 - 05:05 : elias1884
please overthink the revision system default workflow as well. don't
look at the revision system as an isolated system but as a part of the
whole workflow system!
if you combine revisions with the moderation queue the most logic
default workflow would be like that:
auth user creates node (revision #0)
admin approves the node (status = 1, moderation = 0)
=> node publicly available
auth user finds typo and changes node (revision #1, status = 0,
moderation = 1)
-------
what happens at that point at the moment is, that the node is not
accessible anymore at all until the new revision is approved by admin.
of course the new revision should not go online until reviewed and
approved, this is absolutely correct, but there is no reason to not
take the old revision offline, since it was already approved and should
therefore be online until the new revision is approved. it is not
practical if a node disappears only because the author corrected a
typo.
-------
admin approves the node (status = 1, moderation = 0)
eventhough I first thought a plain boolean active field would not be
capable of providing that functionality if finally came to the
conclusion, that it can. The only thing to do is to not set that bit,
when a new revision is created, but when it is approved (in case
moderation is activated under default workflow). Every revision should
have its own moderation, status and active field and on approval they
are set like this (status=1, moderation=0, active=Y).
When you wanna rollback to an old revision, you can chose between all
revisions that already have the moderation bit set back to 0 again and
the published to 1. There should be an extra permission for rollback!
another concern that I have about the default workflow is, that users
can't see the content, they have just created, when moderation is
enabled. Eventhough, there is a big fat "submission accepted" presented
after submissions, unexperienced users tend to question the information
those stupid tincans give them, if they can't find their content
afterwards. Many users are really lazy bastards and they don't even
read the status messages. The best feedback about whether his story was
submitted successfully or not of course is, if he can find the story
somewhere on the site, maybe with a status message on top of it,
mentioning, that the content is currently not publicly available since
it has not been approved yet. there should be a my content section
under my account, like somebody is trying to do with the workspace
module I guess.
so my suggestion is to make (status=0, moderation=1) still available
for the creator under a my content section somewhere!
------------------------------------------------------------------------
November 24, 2004 - 04:21 : Nick Nassar
I agree. The current workflow for moderation queues and revisions needs
to change, but this patch isn't the place for it. The patch is already
too big, and it only does the backend stuff.
Instead of adding more to this patch and making it take even longer to
get into core, would you mind creating a new issue for your UI
suggestions, so the those changes can be added as a separate patch?
Thanks,
Nick
------------------------------------------------------------------------
December 11, 2004 - 12:26 : Dries
This patch is _much_ needed so I'd love to see someone revive it. In
order for this patch to be accepted, the following needs to be done:
Update this patch to CVS HEAD.
Rename revid to vid.
Rename node_rev to node_revisions.
Rename node_rev.changed to node_revisions.timestamp.
Rename $rnode to $revision.
Fix the coding style to match Drupal's: proper spacing, single quotes
where possible, proper variable names.
Benchmark this patch with a large database with enough revisions. I'd
be happy to benchmark this on my local copy of the drupal.org database.
The book.log field should probably move to the node_revisions table.
This can be done in a separate patch.
Investigate whether transactions are well-supported.
------------------------------------------------------------------------
December 13, 2004 - 00:25 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004-r… (11.02 KB)
I've worked a bit on the patch (coding style issues as mentioned by
Dries). One thing I noticed is that the patch uses REPLACE. IIRC this
needs to be chagned to "UPDATE, if fail INSERT" for pgsql
compatibility.
Nick, are you still interested in working on that patch? I'd like to
know how it works on your site and work on getting it into core.
------------------------------------------------------------------------
December 13, 2004 - 12:33 : Dries
Gerhard: your patch does not apply.
------------------------------------------------------------------------
December 13, 2004 - 17:10 : killes(a)www.drop.org
Yes, I know, that was the same version as I mailed to you earlier.
------------------------------------------------------------------------
December 13, 2004 - 21:02 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions.patch (52.96 KB)
Ok, upüdated the patch to cvs.
------------------------------------------------------------------------
December 14, 2004 - 08:58 : Dries
Some more comments:
db_begin_transaction() and db_end_transaction() do not belong in
database.inc, but in database.mysql.inc and database.pgsql.inc
respectively.
The node module calls node_revisionsision_list() which is not defined.
(Fxed that on my local copy.)
Do db_begin_transaction() and db_end_transaction() deprecate Jeremy's
table locking patch?
The upgrade path assigns the wrong user ID to each revision.
The upgrade path assigns the wrong date to each revision (that or a
node's revision page shows the wrong usernames/dates).
The coding style needs a bit of work, but we can worry about that
later.
------------------------------------------------------------------------
December 14, 2004 - 17:34 : Nick Nassar
If you need any help getting those things fixed, just let me know.
------------------------------------------------------------------------
December 14, 2004 - 17:50 : Nick Nassar
How this relates to Jeremy's node locking patch:
There was lots of discussion, and node locking was decided against
because from an end user point of view you never want a node to be
locked. He's now advocating for a much simpler patch that warns users
if their changes will overwrite someone elses. That patch still has a
race condition, which might be fixed using db_begin_transaction().
http://drupal.org/node/6025
------------------------------------------------------------------------
December 14, 2004 - 22:26 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_0.patch (55.96 KB)
Here is an updated patch that tries to address Dries concerns.
------------------------------------------------------------------------
December 15, 2004 - 08:32 : Dries
Attachment: http://drupal.org/files/issues/revisions-bug.png (76.06 KB)
It didn't fix the aforementioned bugs. See attached screenshot.
------------------------------------------------------------------------
January 6, 2005 - 20:15 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_1.patch (51.77 KB)
Ok, here is a new version. Dries and myself worked hart at it, so please
have a look.
what is still missing
- database upgrades for the core modules with an own table
- contrib modules need an upgrade too.
- do we need nid and vid in both the node and the node_revisions table?
- the amount of sql queries means a good stress testing for large
databases.
------------------------------------------------------------------------
January 19, 2005 - 21:43 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_2.patch (49.49 KB)
Here is an updated patch. We discussed to keep the current title in node
module and also in the revisiosn table. This is content duplication but
will save many joins as many queries only need the title of a node.
Discussion is welcome.
------------------------------------------------------------------------
January 19, 2005 - 23:33 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_3.patch (29.93 KB)
I've implemented the aforementioned solution. This makes the patch much
smaller. The patch now also removes taxonomy_node_has_term() which
wasn't used anywhere. I'd really apprciate if some people could test
drive the patch. It will be another huge improvement for 4.6.
------------------------------------------------------------------------
January 20, 2005 - 00:05 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_4.patch (30 KB)
Another revision. Steven didn't like my literal $node->vid in queries.
------------------------------------------------------------------------
January 20, 2005 - 01:10 : killes(a)www.drop.org
- database upgrades for the core modules with an own table
- contrib modules need an upgrade too.
- do we need nid and vid in both the node and the node_revisions table?
- the amount of sql queries means a good stress testing for large
databases.
These issues are still open, btw. Especially the first one needs to be
tackled.
------------------------------------------------------------------------
January 25, 2005 - 20:11 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_5.patch (51.13 KB)
Here is a patch that has the database tables updated for forum, book,
and page module.
------------------------------------------------------------------------
January 29, 2005 - 22:55 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_6.patch (49.18 KB)
Yet another update to keep it working with head. The patch now also
removes the table definitons for the page table.
------------------------------------------------------------------------
January 29, 2005 - 22:57 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_7.patch (55.69 KB)
Sorry, that was the old version, this is the right one.
------------------------------------------------------------------------
January 31, 2005 - 19:55 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_8.patch (55.71 KB)
Updated once more.
------------------------------------------------------------------------
January 31, 2005 - 20:52 : Dries
Anyone to help review/test this?
------------------------------------------------------------------------
January 31, 2005 - 21:22 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_9.patch (49.29 KB)
Updated again, the update functions occurred twice. Thanks Bart.
------------------------------------------------------------------------
February 2, 2005 - 00:27 : killes(a)www.drop.org
Don't know if the db I am using is corrupted or what. I still do have
some didficulties.
The latest patch is attached.
------------------------------------------------------------------------
February 2, 2005 - 00:27 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_10.patch (49.67 KB)
I am probably slowly going mad ...
------------------------------------------------------------------------
February 2, 2005 - 01:54 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_11.patch (48.95 KB)
The update issue still needs investigating. This patch is updated for
cvs.
------------------------------------------------------------------------
February 2, 2005 - 20:20 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_12.patch (49.83 KB)
Ok, here is a new version. I've solved my troubles with book.module.
There are still some issues with forum module. Possibly due to
inconsistent database.
------------------------------------------------------------------------
February 2, 2005 - 21:31 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_13.patch (49.83 KB)
Turns out the drupal.org database had indeed some quirks. Please run
this query in your oldest db and tell me the result:
select nid,type from node where type like '%/%';
If you get a non-zero result we might need to add another security
update.
The patch could use still more testing, though.
------------------------------------------------------------------------
February 3, 2005 - 01:16 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_14.patch (49 KB)
Ok, we are getting somewhere. At a first glance the update is working.
There is a problem remaining: the revisions tab will be shown whether
the node has revisions or not. Not sure we can/need to fix this.
People with a drupal.org account can log in at
http://killes.drupaldevs.org/revision/ and poke around. Your
permissions will be the same as on drupal.org. Feel free to vreak
everything but don't forget to file complaints here. (Note: this is
only a pruned version of the drupal.org database with all project nodes
and nodes with nids > 7000 dropped).
------------------------------------------------------------------------
February 3, 2005 - 04:19 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_15.patch (52.39 KB)
There was some error in node_save and also the patches to the
database.inc files got lost...
------------------------------------------------------------------------
February 3, 2005 - 07:07 : robertDouglass
Submitting book pages doesn't work on your test site. It puts the entire
content of the preview inside the body textarea. I wrote a sentence in
the body and the log, and pressing preview put several lines of HTML
containing both sentences in the body textarea on the preview page,
plus the book page wouldn't submit.
-R
------------------------------------------------------------------------
February 3, 2005 - 07:50 : Junyor
0 results here. I started using Drupal with version 4.4, though.
------------------------------------------------------------------------
February 3, 2005 - 23:56 : killes(a)www.drop.org
@Junyor: Thanks, that's a good sign. Maybe somebody else has an older db
to try.
@robertDouglass: The first effect you describe is due to drupaldevs
running on PHP 5. I am unsure why the second thing does not work. In
node_save() the node object has a nid although there is none in the
form. Very strange.
I've enabled display of db queries on the testsite.
------------------------------------------------------------------------
February 4, 2005 - 19:17 : dmjossel
No results here on the query:
select nid,type from node where type like '%/%';
On a database that was put in place prior to Drupal 4 and is now
running on 4.5.2.
------------------------------------------------------------------------
February 4, 2005 - 20:44 : killes(a)www.drop.org
@dmjossel: thanks.
@all. The strange problem I reported was apparently php 5 related.
After applying Steven's php 5 patch it went away. One error is
remaining: If I create a new forum topic it will be shown as part of
the book on preview. Hmm, that was due to a db that got corrupted
during testing so that is fixed too.
Please poke around at the test site and look if you find more errors.
------------------------------------------------------------------------
February 5, 2005 - 07:16 : Steven
By the way, I just remembered that Drupal.org has some blogs lingering
on in the database even though blog.module is not enabled. Perhaps this
is causing troubles?
------------------------------------------------------------------------
February 5, 2005 - 11:22 : Anonymous
I can't see why it would. Drupal.org will need extra updates for images
and project nodes because those have their own tables. GK.
------------------------------------------------------------------------
February 6, 2005 - 12:49 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_16.patch (52.49 KB)
Updated to apply to cvs again.
------------------------------------------------------------------------
February 22, 2005 - 20:15 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_17.patch (49.64 KB)
Updated again.
All we need is a patch to upload module and an upgrade path for it.
------------------------------------------------------------------------
March 4, 2005 - 04:22 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_18.patch (52.31 KB)
Updated once more. Moved log field from book to node_revisions table as
discussed in Antwerp. upload module still missing.
We need to decide under which circumstances the log field should be
displayed. Should that be added to the workflow? Should it depend on
the revisions setting?
------------------------------------------------------------------------
March 5, 2005 - 19:27 : Anonymous
Attachment: http://drupal.org/files/issues/revisions_20.patch (75.52 KB)
Ok, here it is: Yet another revision of this grrrrreat patch.
Changes from previous versions:
- supports versioning for uploaded files. A problem is that if you
delete a file, it will be gone for all revisions.
- the log field is now in the node_revsions table, but each module has
to decide whether to show it or not.
I've implemented it for the page and the book type odes. Also, the
field can be edited when adding non-book nodes to the book. The log is
displayed on the revisions page and if a node is moderated.
- the revisions are moved to an old_revisions table to a) get the node
table smaller and b) still leave the mavailable for contrib modules
that want to retreive old version data.
The patch has been applied to killes.drupaldevs.org/revision where it
can be tested by anybody (especially people who have "site admin"
rights on drupal.org)
The database is from drupal.org and you should b able to log in with
your pass or simply mail yourself a new one.
Gerhard
------------------------------------------------------------------------
March 5, 2005 - 19:51 : Anonymous
Attachment: http://drupal.org/files/issues/revisions_21.patch (59.42 KB)
BTW, I marked this a bug because atm the revisions field can grow quite
big. Neil has reported problems from some users who were not able to
load some nodes due to to many large revisions.
Also, som unrelated stuff crept into the patch. New version attached.
------------------------------------------------------------------------
March 8, 2005 - 05:56 : killes(a)www.drop.org
Attachment: http://drupal.org/files/issues/revisions_22.patch (60.29 KB)
Ok, I think I got it.
Changes to last version:
- uploads are no properly versioned.
Missing are still pgsql checks and updates.
------------------------------------------------------------------------
March 10, 2005 - 16:58 : Anonymous
Was able to get http://drupal.org/files/issues/revisions_21.patch to
work with drupal-cvs.tar.gz (10 March 2005) by:
- includes/database.mysql.inc: Commenting out duplicates for functions
function db_begin_transaction and function db_commit_transaction
- modules: node.module: Removing "'title' => $node->title," from
$node_table_values variable declaration and removing "'title' =>
"'%s'"," from "$node_table_types" variable declaration.
Happy to submit a patch if requested. I'll watch this thread.
------------------------------------------------------------------------
March 11, 2005 - 23:59 : killes(a)www.drop.org
The duplicate function has been removed in rev 22 of this patch.
Why do you think the changes in node_save are needed? Titles are saved
in both tables for performance reasons.
1
0
Issue status update for http://drupal.org/node/18700
Project: Drupal
Version: cvs
Component: base system
Category: bug reports
Priority: critical
Assigned to: stefan nagtegaal
Reported by: stefan nagtegaal
Updated by: Dries
Status: patch
I'm OK with this! I'd like to await James' input though.
Dries
Previous comments:
------------------------------------------------------------------------
March 10, 2005 - 20:13 : stefan nagtegaal
The idea behind the image.gd.inc file is that it should be usable for
people who are using GD 1 and GD 2.
Unfortunatly, the way it is written now, only support for GD 2 can be
given because:
Current way of determining GD 2, is not right:
if (function_exists('imageCreateTrueColor')) {
// GD 2 Handling;
}
else {
// GD 1 Handling
}
this doesn't work.. imageCreateTrueColor() was already implemented in
GD 1, only it failed to work properly..
See these pages:
http://www.php.net/imagecratetruecolor#25234 &
http://www.php.net/imagecratetruecolor#25487
According to this comments, we should implement this like this:
// Silence errors using the @
$image = @imageCreateTrueColor(......);
if (!$image) {
// GD 1 Handling;
}
------------------------------------------------------------------------
March 11, 2005 - 20:03 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image-inc.patch (2.26 KB)
Attached patch improves:
- the way which deals with the detection of GD 1 or GD 2;
- error messages as reported by Steven here:
http://drupal.org/node/17645;
After applying this to HEAD/Drupal 4.6 RC, we can close the following
issues:
- http://drupal.org/node/17645;
- http://drupal.org/node/13027;
Please test/comment and apply attached patch..
------------------------------------------------------------------------
March 12, 2005 - 10:41 : Dries
Isn't the approach taking in http://drupal.org/node/13027 nicer? It
doesn't surpress warnings/errors. Maybe that approach doesn't work?
------------------------------------------------------------------------
March 12, 2005 - 10:50 : Bèr Kessels
-1 on supressing errors with @. Its ugly, and hardly used in Drupal. We
should use our own error handling at all times.
------------------------------------------------------------------------
March 12, 2005 - 11:42 : stefan nagtegaal
I will make a new patch, with the more elegant approach you mentioned
Dries...
------------------------------------------------------------------------
March 12, 2005 - 19:05 : stefan nagtegaal
As noticed before in this issue.. The function imageCreateTrueColor
_does_ exist, even in versions of GD before 1.8 only it was broken..
So, function_exist() doesn't work on that the function, because it
really _does_ exist, but the fact is that it's simply broken..
So unfortunatly, the more elegant solution doesn't seem to work..
------------------------------------------------------------------------
March 12, 2005 - 20:01 : pz
Stupid question, why not just use the gd_info function?
------------------------------------------------------------------------
March 13, 2005 - 10:54 : Dries
What versions of PHP have those old GD toolkits?
PHP 4.3.10 comes with GD 2.0.28, it seems, as does PHP 5.0.3.
PHP 4.3.3, Drupal's current minimum PHP requirement, comes with GD
2.0.15.
>From the looks, we don't need GD 1 handling at all ... (but we might
need a global version check).
Thoughts?
------------------------------------------------------------------------
March 13, 2005 - 12:32 : stefan nagtegaal
PHP 4.1.2 comes with GD 1.62..
The nasty/hackish code with the build-in GD toolkit, is exactly why I
proposed to split the image.inc into a separate image.gd1.inc and
image.gd2.inc.
I would propose a patch which does:
- remove all the GD < 2 function calls like imageCreate() and
imageCopyResized() and have a real GD 2 compatible image.inc;
- introduce a new image.gd1.inc file into Walkahs sandbox; If the
module is being moved to the contributions, the image.gd1.inc is also
available for people without proper GD 2 support;
- more userfriendly and helpfull error reporting for the image-api..
If we agree about this, I'll make patches for inclusion into 4.6..
Which does contain the before mentioned problems with the current
usage..
Please, I would like to have some feedback about this...
------------------------------------------------------------------------
March 13, 2005 - 14:55 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image-inc_0.patch (2.99 KB)
As proposed above, patch for image.inc..
Please comment or apply...
The image.gd1.inc file wil be posted after this post...
------------------------------------------------------------------------
March 13, 2005 - 14:57 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image.gd1.inc (2.07 KB)
And the image.gd1.inc file for Walkahs sandbox...
Tell me what you all think about this..
1
0
Issue status update for http://drupal.org/node/11623
Project: Drupal
Version: cvs
Component: base system
-Category: bug reports
+Category: feature requests
Priority: critical
Assigned to: pancho
Reported by: Goba
Updated by: pancho
Status: patch
I feel better if I refile the thread as a feature request. The only
thing you could describe as a bug was that German & Hungarian date
formats were not supported. The rest are definitely feature
improvements.
pancho
Previous comments:
------------------------------------------------------------------------
October 15, 2004 - 19:52 : Goba
Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats.patch (1.99 KB)
Currently if I set a date format for the site, then it will be used with
all interface translations. Different languages however have different
needs when presenting dates. The Hungarian date format does not allow
the day name to be the first and it calls for a dot between the hour
and minute (yes, a dot not a colon). This means that Drupal ships with
no suitable date format for Hungarians. Also none of the Hungarian date
formats will be suitable on an English interface, while none of the
English date formats are acceptable on a Hungarian interface on a
bilingual site.
The only option is to make date formats translateable. Since these are
dynamic t() calls, extractor.php needs to be extended to include the
date formats shipped with drupal in the pot output, so translators will
get them (much like we do for some numbers and day names). I will take
care of extending extractor.php, once this patch gets applied.
------------------------------------------------------------------------
October 15, 2004 - 20:00 : Goba
Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats_0.patch (1.99 KB)
Well, the t() needs to be called on the date format set by the admin, so
I needed to fix three t() calls. Updated patch attached.
------------------------------------------------------------------------
October 15, 2004 - 20:01 : killes(a)www.drop.org
*g* I recall that there was a time when you didn't like this approach.
Anyway, I fully endorse it and would like to see it in 4.5. There is
also no suitable data format for German in core. And yes, that's a bug
not a feature request.
------------------------------------------------------------------------
October 15, 2004 - 20:05 : Goba
Yes, I didn't like it, because these are dynamic t()s. But since the
selected format always needs to be presented in the actual language, it
needs to be dynamic by nature. We have some of these dynamic t()s
already, handled specially in extractor.php.
------------------------------------------------------------------------
October 16, 2004 - 00:07 : Steven
I like this idea a lot, but I don't like the simplistic "wrap it all in
t" feature. There are instances where format_date output is intended
for script output. Changing those date formats would mean a bug in the
code or UI (for example the way a node date is specified and parsed).
Only those strings that are intended for user consumption should be
translated. That's why I think we need a more specialized date handling
component. It would deal with stuff like variations ('show hour/hide
hour'), long/short variations (we need to review the usage of
small/medium/long), as well as localizing them.
We would need to revise the formats shipped with Drupal, but this gets
tricky. For example would we only ship US-style AM/PM date formats with
core? Should we only have M/D/Y ordering, or also D/M/Y ordering? If we
move the time format settings to locale (where they do below, in
principle) we might get a drop in usability: moving from a simple
selection box, to requiring the whole localisation framework 'just to
switch from 12 hours to 24 hours'.
In some parts of the world, there is only one format for dates (e.g.
chinese/japanese 2004年10月16日 1時00分). For them, the only
variations are long vs short (whether to show the year, the time, etc),
in other parts of the world all bets are off and people use what they
like.
This would mean that if we provide a default set of formats, and let
them be translated through PO files, some languages might require more
items than provided and some languages would have to use duplicates to
fill all available options. On top of that, there is no guarantee that
format #3 in French matches format #3 in German, so you'd need to be
able to specify this per language.
Ideally, we would let a language define any number of date formats, and
let the user choose one of those per language. We could also provide a
custom option where you enter your own date format, for the realy picky
ones.
The question remains: what do we do for sites without locale enabled?
We can't go to requiring locale just to switch the date on a
non-localized site. In that case, perhaps we could do this:
- Have locale.module deal with date formats when it is enabled. Have a
per-language format setting, selected from a language-specific,
po-defined date format list.
- Have system.module implement a 'localization' admin menu item if
locale.module is /not/ enabled. This page carries the old selector,
which acts like a simplified version of locale's format selector, with
the list we current have in Drupal (i.e. variations of 24/12hours,
d/m/y, y/m/d, m/d/y, ...). To prevent confusion, we add a link to
system.module's version of the localization page, which instructs users
to enable locale.module for full localization abilities).
I know it sounds a bit weird (we certainly don't do this anywhere
else), but it would be consistent in terms of UI (the date format is
"localization" no matter how you look at it) and keep both
non-localized site admins and localized site admins happy, without
sacrificing too much ease of set up.
------------------------------------------------------------------------
October 16, 2004 - 10:34 : Goba
Steven, you say:
"/There are instances where format_date output is intended for script
output. Changing those date formats would mean a bug in the code or UI/
"
But the current approach is to translate the month and day names in
every case, so format_date **already** assumes that the date it outputs
is for human consumption and not for machines. So changing the order of
the fields in user date formats should not hurt. Sure it might not be a
useability dream, but it does not hurt.
By the way, adjusting date formats based on user language is only a
small piece of localisation not covered by Drupal core yet. I need to
use one mission statement and site name in all languages, and even my
primary and secondary links need to have the same text in all site
languages, since they are not translateable. If we are about to find a
generic solution for locale covered settings, we need to take these
into account too.
------------------------------------------------------------------------
October 29, 2004 - 01:10 : Steven
""so format_date *already* assumes that the date it outputs is for human
consumption and not for machines."
"
What I mean is changing the placement of the date components (day,
month, year). For numerical dates which are parsed by a program, this
will introduce bugs. I'd much rather have a cleaner date format
selection method integrated with locale.
------------------------------------------------------------------------
October 29, 2004 - 14:19 : Goba
Steven, the custom date formats are not translated, only those selected
by the admin. And the admin can select date types with spelled out
month/day names at any level, so they are not machine parsable anyway
in the current Drupal version. **This patch does not introduce bugs.**
I admit it does not look good, but there is no system for locale
dependant admin settings yet.
------------------------------------------------------------------------
October 29, 2004 - 16:05 : Steven
I'm saying we do need such a system.
With your patch, the format that the admin selects in language A will
not necessarily match the one in language B. We should go for
locale-defined formats with a locale-neutral list to fallback on.
------------------------------------------------------------------------
March 13, 2005 - 15:55 : pancho
Attachment: http://drupal.org/files/issues/dateformat.patch.zip (14.91 KB)
************************
DATEFORMAT-PATCH PACKAGE
************************
by pancho(a)suenderhauf.net
2005-03-13 03:07
This 'DateFormat' patch package only works with the latest CVS version
of Drupal 4.60 RC.
This 'DateFormat' patch package applies the following changes:
1.
It introduces a more flexible system of standard date and time formats
which helps both users and developers while staying
backwards-compatible. We often see special date formats be used in
modules, which is only second best, as these can neither
be adapted by the user nor localized. More flexibility allows the
developers to use the built-in format functions wherever
possible.
Changes in detail:
- The standard date formats used to be combined date&time-formats. This
is now split into separate date formats and time
formats, to allow dates and times to be rendered in a more flexible
way.
- Still it's backward compatible: the combined formats - while
deprecated - still exist.
- A "Tiny" format is added to the "Small", "Medium" and "Large"
formats.
2.
It enables Drupal to show e.g. German and Hungarian date formats (with
a colon as separator), which was _not_ possible
before.
3.
It prepares both the Drupal system and module developers for the
upcoming locale_dateformat module. This module will fully
localize date and time formats according to the localization of the
current user. The locale_dateformat module will be
available within the next weeks.
I'm always happy about suggestions and criticism, if constructive. You
can either write a comment on www.cvs.net or an email
to pancho(a)suenderhauf.net.
Bye, Pancho
-------------
INSTALLATION:
-------------
The package contains two patches for the latest CVS versions of Drupal
4.60 RC, both of which should be applied:
1. common.inc (patch for v.1.429)
2. system.module (patch for v.1.197)
There are no changes in database structure.
------------------------------------------------------------------------
March 13, 2005 - 15:59 : pancho
P.S. What the hell did I write about www.cvs.net? No, what I meant was
of course www.drupal.org (here), where you can write a comment!
1
0
Issue status update for http://drupal.org/node/11623
Project: Drupal
Version: cvs
Component: base system
Category: bug reports
Priority: critical
Assigned to: pancho
Reported by: Goba
Updated by: pancho
Status: patch
P.S. What the hell did I write about www.cvs.net? No, what I meant was
of course www.drupal.org (here), where you can write a comment!
pancho
Previous comments:
------------------------------------------------------------------------
October 15, 2004 - 19:52 : Goba
Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats.patch (1.99 KB)
Currently if I set a date format for the site, then it will be used with
all interface translations. Different languages however have different
needs when presenting dates. The Hungarian date format does not allow
the day name to be the first and it calls for a dot between the hour
and minute (yes, a dot not a colon). This means that Drupal ships with
no suitable date format for Hungarians. Also none of the Hungarian date
formats will be suitable on an English interface, while none of the
English date formats are acceptable on a Hungarian interface on a
bilingual site.
The only option is to make date formats translateable. Since these are
dynamic t() calls, extractor.php needs to be extended to include the
date formats shipped with drupal in the pot output, so translators will
get them (much like we do for some numbers and day names). I will take
care of extending extractor.php, once this patch gets applied.
------------------------------------------------------------------------
October 15, 2004 - 20:00 : Goba
Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats_0.patch (1.99 KB)
Well, the t() needs to be called on the date format set by the admin, so
I needed to fix three t() calls. Updated patch attached.
------------------------------------------------------------------------
October 15, 2004 - 20:01 : killes(a)www.drop.org
*g* I recall that there was a time when you didn't like this approach.
Anyway, I fully endorse it and would like to see it in 4.5. There is
also no suitable data format for German in core. And yes, that's a bug
not a feature request.
------------------------------------------------------------------------
October 15, 2004 - 20:05 : Goba
Yes, I didn't like it, because these are dynamic t()s. But since the
selected format always needs to be presented in the actual language, it
needs to be dynamic by nature. We have some of these dynamic t()s
already, handled specially in extractor.php.
------------------------------------------------------------------------
October 16, 2004 - 00:07 : Steven
I like this idea a lot, but I don't like the simplistic "wrap it all in
t" feature. There are instances where format_date output is intended
for script output. Changing those date formats would mean a bug in the
code or UI (for example the way a node date is specified and parsed).
Only those strings that are intended for user consumption should be
translated. That's why I think we need a more specialized date handling
component. It would deal with stuff like variations ('show hour/hide
hour'), long/short variations (we need to review the usage of
small/medium/long), as well as localizing them.
We would need to revise the formats shipped with Drupal, but this gets
tricky. For example would we only ship US-style AM/PM date formats with
core? Should we only have M/D/Y ordering, or also D/M/Y ordering? If we
move the time format settings to locale (where they do below, in
principle) we might get a drop in usability: moving from a simple
selection box, to requiring the whole localisation framework 'just to
switch from 12 hours to 24 hours'.
In some parts of the world, there is only one format for dates (e.g.
chinese/japanese 2004年10月16日 1時00分). For them, the only
variations are long vs short (whether to show the year, the time, etc),
in other parts of the world all bets are off and people use what they
like.
This would mean that if we provide a default set of formats, and let
them be translated through PO files, some languages might require more
items than provided and some languages would have to use duplicates to
fill all available options. On top of that, there is no guarantee that
format #3 in French matches format #3 in German, so you'd need to be
able to specify this per language.
Ideally, we would let a language define any number of date formats, and
let the user choose one of those per language. We could also provide a
custom option where you enter your own date format, for the realy picky
ones.
The question remains: what do we do for sites without locale enabled?
We can't go to requiring locale just to switch the date on a
non-localized site. In that case, perhaps we could do this:
- Have locale.module deal with date formats when it is enabled. Have a
per-language format setting, selected from a language-specific,
po-defined date format list.
- Have system.module implement a 'localization' admin menu item if
locale.module is /not/ enabled. This page carries the old selector,
which acts like a simplified version of locale's format selector, with
the list we current have in Drupal (i.e. variations of 24/12hours,
d/m/y, y/m/d, m/d/y, ...). To prevent confusion, we add a link to
system.module's version of the localization page, which instructs users
to enable locale.module for full localization abilities).
I know it sounds a bit weird (we certainly don't do this anywhere
else), but it would be consistent in terms of UI (the date format is
"localization" no matter how you look at it) and keep both
non-localized site admins and localized site admins happy, without
sacrificing too much ease of set up.
------------------------------------------------------------------------
October 16, 2004 - 10:34 : Goba
Steven, you say:
"/There are instances where format_date output is intended for script
output. Changing those date formats would mean a bug in the code or UI/
"
But the current approach is to translate the month and day names in
every case, so format_date **already** assumes that the date it outputs
is for human consumption and not for machines. So changing the order of
the fields in user date formats should not hurt. Sure it might not be a
useability dream, but it does not hurt.
By the way, adjusting date formats based on user language is only a
small piece of localisation not covered by Drupal core yet. I need to
use one mission statement and site name in all languages, and even my
primary and secondary links need to have the same text in all site
languages, since they are not translateable. If we are about to find a
generic solution for locale covered settings, we need to take these
into account too.
------------------------------------------------------------------------
October 29, 2004 - 01:10 : Steven
""so format_date *already* assumes that the date it outputs is for human
consumption and not for machines."
"
What I mean is changing the placement of the date components (day,
month, year). For numerical dates which are parsed by a program, this
will introduce bugs. I'd much rather have a cleaner date format
selection method integrated with locale.
------------------------------------------------------------------------
October 29, 2004 - 14:19 : Goba
Steven, the custom date formats are not translated, only those selected
by the admin. And the admin can select date types with spelled out
month/day names at any level, so they are not machine parsable anyway
in the current Drupal version. **This patch does not introduce bugs.**
I admit it does not look good, but there is no system for locale
dependant admin settings yet.
------------------------------------------------------------------------
October 29, 2004 - 16:05 : Steven
I'm saying we do need such a system.
With your patch, the format that the admin selects in language A will
not necessarily match the one in language B. We should go for
locale-defined formats with a locale-neutral list to fallback on.
------------------------------------------------------------------------
March 13, 2005 - 15:55 : pancho
Attachment: http://drupal.org/files/issues/dateformat.patch.zip (14.91 KB)
************************
DATEFORMAT-PATCH PACKAGE
************************
by pancho(a)suenderhauf.net
2005-03-13 03:07
This 'DateFormat' patch package only works with the latest CVS version
of Drupal 4.60 RC.
This 'DateFormat' patch package applies the following changes:
1.
It introduces a more flexible system of standard date and time formats
which helps both users and developers while staying
backwards-compatible. We often see special date formats be used in
modules, which is only second best, as these can neither
be adapted by the user nor localized. More flexibility allows the
developers to use the built-in format functions wherever
possible.
Changes in detail:
- The standard date formats used to be combined date&time-formats. This
is now split into separate date formats and time
formats, to allow dates and times to be rendered in a more flexible
way.
- Still it's backward compatible: the combined formats - while
deprecated - still exist.
- A "Tiny" format is added to the "Small", "Medium" and "Large"
formats.
2.
It enables Drupal to show e.g. German and Hungarian date formats (with
a colon as separator), which was _not_ possible
before.
3.
It prepares both the Drupal system and module developers for the
upcoming locale_dateformat module. This module will fully
localize date and time formats according to the localization of the
current user. The locale_dateformat module will be
available within the next weeks.
I'm always happy about suggestions and criticism, if constructive. You
can either write a comment on www.cvs.net or an email
to pancho(a)suenderhauf.net.
Bye, Pancho
-------------
INSTALLATION:
-------------
The package contains two patches for the latest CVS versions of Drupal
4.60 RC, both of which should be applied:
1. common.inc (patch for v.1.429)
2. system.module (patch for v.1.197)
There are no changes in database structure.
1
0
Issue status update for http://drupal.org/node/11623
Project: Drupal
-Version: 4.5.0
+Version: cvs
Component: base system
Category: bug reports
-Priority: normal
+Priority: critical
-Assigned to: Goba
+Assigned to: pancho
Reported by: Goba
Updated by: pancho
Status: patch
Attachment: http://drupal.org/files/issues/dateformat.patch.zip (14.91 KB)
************************
DATEFORMAT-PATCH PACKAGE
************************
by pancho(a)suenderhauf.net
2005-03-13 03:07
This 'DateFormat' patch package only works with the latest CVS version
of Drupal 4.60 RC.
This 'DateFormat' patch package applies the following changes:
1.
It introduces a more flexible system of standard date and time formats
which helps both users and developers while staying
backwards-compatible. We often see special date formats be used in
modules, which is only second best, as these can neither
be adapted by the user nor localized. More flexibility allows the
developers to use the built-in format functions wherever
possible.
Changes in detail:
- The standard date formats used to be combined date&time-formats. This
is now split into separate date formats and time
formats, to allow dates and times to be rendered in a more flexible
way.
- Still it's backward compatible: the combined formats - while
deprecated - still exist.
- A "Tiny" format is added to the "Small", "Medium" and "Large"
formats.
2.
It enables Drupal to show e.g. German and Hungarian date formats (with
a colon as separator), which was _not_ possible
before.
3.
It prepares both the Drupal system and module developers for the
upcoming locale_dateformat module. This module will fully
localize date and time formats according to the localization of the
current user. The locale_dateformat module will be
available within the next weeks.
I'm always happy about suggestions and criticism, if constructive. You
can either write a comment on www.cvs.net or an email
to pancho(a)suenderhauf.net.
Bye, Pancho
-------------
INSTALLATION:
-------------
The package contains two patches for the latest CVS versions of Drupal
4.60 RC, both of which should be applied:
1. common.inc (patch for v.1.429)
2. system.module (patch for v.1.197)
There are no changes in database structure.
pancho
Previous comments:
------------------------------------------------------------------------
October 15, 2004 - 19:52 : Goba
Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats.patch (1.99 KB)
Currently if I set a date format for the site, then it will be used with
all interface translations. Different languages however have different
needs when presenting dates. The Hungarian date format does not allow
the day name to be the first and it calls for a dot between the hour
and minute (yes, a dot not a colon). This means that Drupal ships with
no suitable date format for Hungarians. Also none of the Hungarian date
formats will be suitable on an English interface, while none of the
English date formats are acceptable on a Hungarian interface on a
bilingual site.
The only option is to make date formats translateable. Since these are
dynamic t() calls, extractor.php needs to be extended to include the
date formats shipped with drupal in the pot output, so translators will
get them (much like we do for some numbers and day names). I will take
care of extending extractor.php, once this patch gets applied.
------------------------------------------------------------------------
October 15, 2004 - 20:00 : Goba
Attachment: http://drupal.org/files/issues/Drupal-translate-date-formats_0.patch (1.99 KB)
Well, the t() needs to be called on the date format set by the admin, so
I needed to fix three t() calls. Updated patch attached.
------------------------------------------------------------------------
October 15, 2004 - 20:01 : killes(a)www.drop.org
*g* I recall that there was a time when you didn't like this approach.
Anyway, I fully endorse it and would like to see it in 4.5. There is
also no suitable data format for German in core. And yes, that's a bug
not a feature request.
------------------------------------------------------------------------
October 15, 2004 - 20:05 : Goba
Yes, I didn't like it, because these are dynamic t()s. But since the
selected format always needs to be presented in the actual language, it
needs to be dynamic by nature. We have some of these dynamic t()s
already, handled specially in extractor.php.
------------------------------------------------------------------------
October 16, 2004 - 00:07 : Steven
I like this idea a lot, but I don't like the simplistic "wrap it all in
t" feature. There are instances where format_date output is intended
for script output. Changing those date formats would mean a bug in the
code or UI (for example the way a node date is specified and parsed).
Only those strings that are intended for user consumption should be
translated. That's why I think we need a more specialized date handling
component. It would deal with stuff like variations ('show hour/hide
hour'), long/short variations (we need to review the usage of
small/medium/long), as well as localizing them.
We would need to revise the formats shipped with Drupal, but this gets
tricky. For example would we only ship US-style AM/PM date formats with
core? Should we only have M/D/Y ordering, or also D/M/Y ordering? If we
move the time format settings to locale (where they do below, in
principle) we might get a drop in usability: moving from a simple
selection box, to requiring the whole localisation framework 'just to
switch from 12 hours to 24 hours'.
In some parts of the world, there is only one format for dates (e.g.
chinese/japanese 2004年10月16日 1時00分). For them, the only
variations are long vs short (whether to show the year, the time, etc),
in other parts of the world all bets are off and people use what they
like.
This would mean that if we provide a default set of formats, and let
them be translated through PO files, some languages might require more
items than provided and some languages would have to use duplicates to
fill all available options. On top of that, there is no guarantee that
format #3 in French matches format #3 in German, so you'd need to be
able to specify this per language.
Ideally, we would let a language define any number of date formats, and
let the user choose one of those per language. We could also provide a
custom option where you enter your own date format, for the realy picky
ones.
The question remains: what do we do for sites without locale enabled?
We can't go to requiring locale just to switch the date on a
non-localized site. In that case, perhaps we could do this:
- Have locale.module deal with date formats when it is enabled. Have a
per-language format setting, selected from a language-specific,
po-defined date format list.
- Have system.module implement a 'localization' admin menu item if
locale.module is /not/ enabled. This page carries the old selector,
which acts like a simplified version of locale's format selector, with
the list we current have in Drupal (i.e. variations of 24/12hours,
d/m/y, y/m/d, m/d/y, ...). To prevent confusion, we add a link to
system.module's version of the localization page, which instructs users
to enable locale.module for full localization abilities).
I know it sounds a bit weird (we certainly don't do this anywhere
else), but it would be consistent in terms of UI (the date format is
"localization" no matter how you look at it) and keep both
non-localized site admins and localized site admins happy, without
sacrificing too much ease of set up.
------------------------------------------------------------------------
October 16, 2004 - 10:34 : Goba
Steven, you say:
"/There are instances where format_date output is intended for script
output. Changing those date formats would mean a bug in the code or UI/
"
But the current approach is to translate the month and day names in
every case, so format_date **already** assumes that the date it outputs
is for human consumption and not for machines. So changing the order of
the fields in user date formats should not hurt. Sure it might not be a
useability dream, but it does not hurt.
By the way, adjusting date formats based on user language is only a
small piece of localisation not covered by Drupal core yet. I need to
use one mission statement and site name in all languages, and even my
primary and secondary links need to have the same text in all site
languages, since they are not translateable. If we are about to find a
generic solution for locale covered settings, we need to take these
into account too.
------------------------------------------------------------------------
October 29, 2004 - 01:10 : Steven
""so format_date *already* assumes that the date it outputs is for human
consumption and not for machines."
"
What I mean is changing the placement of the date components (day,
month, year). For numerical dates which are parsed by a program, this
will introduce bugs. I'd much rather have a cleaner date format
selection method integrated with locale.
------------------------------------------------------------------------
October 29, 2004 - 14:19 : Goba
Steven, the custom date formats are not translated, only those selected
by the admin. And the admin can select date types with spelled out
month/day names at any level, so they are not machine parsable anyway
in the current Drupal version. **This patch does not introduce bugs.**
I admit it does not look good, but there is no system for locale
dependant admin settings yet.
------------------------------------------------------------------------
October 29, 2004 - 16:05 : Steven
I'm saying we do need such a system.
With your patch, the format that the admin selects in language A will
not necessarily match the one in language B. We should go for
locale-defined formats with a locale-neutral list to fallback on.
1
0
13 Mar '05
Issue status update for http://drupal.org/node/18700
Project: Drupal
Version: cvs
Component: base system
Category: bug reports
Priority: critical
Assigned to: stefan nagtegaal
Reported by: stefan nagtegaal
Updated by: stefan nagtegaal
Status: patch
Attachment: http://drupal.org/files/issues/image.gd1.inc (2.07 KB)
And the image.gd1.inc file for Walkahs sandbox...
Tell me what you all think about this..
stefan nagtegaal
Previous comments:
------------------------------------------------------------------------
March 10, 2005 - 19:13 : stefan nagtegaal
The idea behind the image.gd.inc file is that it should be usable for
people who are using GD 1 and GD 2.
Unfortunatly, the way it is written now, only support for GD 2 can be
given because:
Current way of determining GD 2, is not right:
if (function_exists('imageCreateTrueColor')) {
// GD 2 Handling;
}
else {
// GD 1 Handling
}
this doesn't work.. imageCreateTrueColor() was already implemented in
GD 1, only it failed to work properly..
See these pages:
http://www.php.net/imagecratetruecolor#25234 &
http://www.php.net/imagecratetruecolor#25487
According to this comments, we should implement this like this:
// Silence errors using the @
$image = @imageCreateTrueColor(......);
if (!$image) {
// GD 1 Handling;
}
------------------------------------------------------------------------
March 11, 2005 - 19:03 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image-inc.patch (2.26 KB)
Attached patch improves:
- the way which deals with the detection of GD 1 or GD 2;
- error messages as reported by Steven here:
http://drupal.org/node/17645;
After applying this to HEAD/Drupal 4.6 RC, we can close the following
issues:
- http://drupal.org/node/17645;
- http://drupal.org/node/13027;
Please test/comment and apply attached patch..
------------------------------------------------------------------------
March 12, 2005 - 09:41 : Dries
Isn't the approach taking in http://drupal.org/node/13027 nicer? It
doesn't surpress warnings/errors. Maybe that approach doesn't work?
------------------------------------------------------------------------
March 12, 2005 - 09:50 : Bèr Kessels
-1 on supressing errors with @. Its ugly, and hardly used in Drupal. We
should use our own error handling at all times.
------------------------------------------------------------------------
March 12, 2005 - 10:42 : stefan nagtegaal
I will make a new patch, with the more elegant approach you mentioned
Dries...
------------------------------------------------------------------------
March 12, 2005 - 18:05 : stefan nagtegaal
As noticed before in this issue.. The function imageCreateTrueColor
_does_ exist, even in versions of GD before 1.8 only it was broken..
So, function_exist() doesn't work on that the function, because it
really _does_ exist, but the fact is that it's simply broken..
So unfortunatly, the more elegant solution doesn't seem to work..
------------------------------------------------------------------------
March 12, 2005 - 19:01 : pz
Stupid question, why not just use the gd_info function?
------------------------------------------------------------------------
March 13, 2005 - 09:54 : Dries
What versions of PHP have those old GD toolkits?
PHP 4.3.10 comes with GD 2.0.28, it seems, as does PHP 5.0.3.
PHP 4.3.3, Drupal's current minimum PHP requirement, comes with GD
2.0.15.
>From the looks, we don't need GD 1 handling at all ... (but we might
need a global version check).
Thoughts?
------------------------------------------------------------------------
March 13, 2005 - 11:32 : stefan nagtegaal
PHP 4.1.2 comes with GD 1.62..
The nasty/hackish code with the build-in GD toolkit, is exactly why I
proposed to split the image.inc into a separate image.gd1.inc and
image.gd2.inc.
I would propose a patch which does:
- remove all the GD < 2 function calls like imageCreate() and
imageCopyResized() and have a real GD 2 compatible image.inc;
- introduce a new image.gd1.inc file into Walkahs sandbox; If the
module is being moved to the contributions, the image.gd1.inc is also
available for people without proper GD 2 support;
- more userfriendly and helpfull error reporting for the image-api..
If we agree about this, I'll make patches for inclusion into 4.6..
Which does contain the before mentioned problems with the current
usage..
Please, I would like to have some feedback about this...
------------------------------------------------------------------------
March 13, 2005 - 13:55 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image-inc_0.patch (2.99 KB)
As proposed above, patch for image.inc..
Please comment or apply...
The image.gd1.inc file wil be posted after this post...
1
0
13 Mar '05
Issue status update for http://drupal.org/node/18700
Project: Drupal
Version: cvs
Component: base system
Category: bug reports
Priority: critical
Assigned to: stefan nagtegaal
Reported by: stefan nagtegaal
Updated by: stefan nagtegaal
Status: patch
Attachment: http://drupal.org/files/issues/image-inc_0.patch (2.99 KB)
As proposed above, patch for image.inc..
Please comment or apply...
The image.gd1.inc file wil be posted after this post...
stefan nagtegaal
Previous comments:
------------------------------------------------------------------------
March 10, 2005 - 19:13 : stefan nagtegaal
The idea behind the image.gd.inc file is that it should be usable for
people who are using GD 1 and GD 2.
Unfortunatly, the way it is written now, only support for GD 2 can be
given because:
Current way of determining GD 2, is not right:
if (function_exists('imageCreateTrueColor')) {
// GD 2 Handling;
}
else {
// GD 1 Handling
}
this doesn't work.. imageCreateTrueColor() was already implemented in
GD 1, only it failed to work properly..
See these pages:
http://www.php.net/imagecratetruecolor#25234 &
http://www.php.net/imagecratetruecolor#25487
According to this comments, we should implement this like this:
// Silence errors using the @
$image = @imageCreateTrueColor(......);
if (!$image) {
// GD 1 Handling;
}
------------------------------------------------------------------------
March 11, 2005 - 19:03 : stefan nagtegaal
Attachment: http://drupal.org/files/issues/image-inc.patch (2.26 KB)
Attached patch improves:
- the way which deals with the detection of GD 1 or GD 2;
- error messages as reported by Steven here:
http://drupal.org/node/17645;
After applying this to HEAD/Drupal 4.6 RC, we can close the following
issues:
- http://drupal.org/node/17645;
- http://drupal.org/node/13027;
Please test/comment and apply attached patch..
------------------------------------------------------------------------
March 12, 2005 - 09:41 : Dries
Isn't the approach taking in http://drupal.org/node/13027 nicer? It
doesn't surpress warnings/errors. Maybe that approach doesn't work?
------------------------------------------------------------------------
March 12, 2005 - 09:50 : Bèr Kessels
-1 on supressing errors with @. Its ugly, and hardly used in Drupal. We
should use our own error handling at all times.
------------------------------------------------------------------------
March 12, 2005 - 10:42 : stefan nagtegaal
I will make a new patch, with the more elegant approach you mentioned
Dries...
------------------------------------------------------------------------
March 12, 2005 - 18:05 : stefan nagtegaal
As noticed before in this issue.. The function imageCreateTrueColor
_does_ exist, even in versions of GD before 1.8 only it was broken..
So, function_exist() doesn't work on that the function, because it
really _does_ exist, but the fact is that it's simply broken..
So unfortunatly, the more elegant solution doesn't seem to work..
------------------------------------------------------------------------
March 12, 2005 - 19:01 : pz
Stupid question, why not just use the gd_info function?
------------------------------------------------------------------------
March 13, 2005 - 09:54 : Dries
What versions of PHP have those old GD toolkits?
PHP 4.3.10 comes with GD 2.0.28, it seems, as does PHP 5.0.3.
PHP 4.3.3, Drupal's current minimum PHP requirement, comes with GD
2.0.15.
>From the looks, we don't need GD 1 handling at all ... (but we might
need a global version check).
Thoughts?
------------------------------------------------------------------------
March 13, 2005 - 11:32 : stefan nagtegaal
PHP 4.1.2 comes with GD 1.62..
The nasty/hackish code with the build-in GD toolkit, is exactly why I
proposed to split the image.inc into a separate image.gd1.inc and
image.gd2.inc.
I would propose a patch which does:
- remove all the GD < 2 function calls like imageCreate() and
imageCopyResized() and have a real GD 2 compatible image.inc;
- introduce a new image.gd1.inc file into Walkahs sandbox; If the
module is being moved to the contributions, the image.gd1.inc is also
available for people without proper GD 2 support;
- more userfriendly and helpfull error reporting for the image-api..
If we agree about this, I'll make patches for inclusion into 4.6..
Which does contain the before mentioned problems with the current
usage..
Please, I would like to have some feedback about this...
1
0
Hi there!
I tried to find contact info for the author of that module to inform him
that the inclusion of the whole fckedtor distro in Drupal contrib CVS
is a violation of the TOS and my intention to remove it.
Please contact me if you disagree.
Cheers,
Gerhard
1
0