For Acquire/Columns I pretty much agree with everything mentioned, changing the num of columns or justification after acquire is a nice "bonus" (some may even find it absolutely necessary). As for CellProviders, as much as I read and understand what was outlined, I cant help but ultimately think that we are blowing this out of proportion and moving closer to a 'Tablerish' design, considering the original intention behind the library. And I'm not talking about the size here. Don't get me wrong its a cool concept and would allow the library to be potentially used for more advanced tips as well and that's exactly my problem. Why does someone that just needs a simple tip with 3-4 columns and maybe updating cells every now and then, have to specify a provider in SetCell (even an integrated/embedded one). Suggestion: We can go with our original idea and break this into 2 versions perhaps.
Why does someone that just needs a simple tip with 3-4 columns and maybe updating cells every now and then, have to specify a provider in SetCell (even an integrated/embedded one).
Actually there are two solutions for basic usage :
1) :AddLine is unchanged, so you could just do AddLine("label1", "label2", "label3")
2) the provider in :SetCell can be omitted, so you could do SetCell(1,1,"Label").
About the library size, Kaelten estimated we shouldn't get over 750 lines with both colspans and CellProvider (not counting comments). I'm inclined to think he is right.
Indeed it can, sorry but that wall of text wasn't actually more "readable" than Tablet documentation (no offense) and it took some time to sink in (perhaps I realized as well that I'm not as smart after all :p). Like I said, I'm not that much concerned about the size, since I do not foresee it will reach excessive or otherwise "unacceptable" amounts. I'm worried about the fact that we may be implementing something that will only be used rarely.
The reason that I want the cellprovider api, is that it can be extremely extensible. With a nice wrapper layer and enough custom providers you could recreate a dewdrop menu system, tablet like behaviors, or anything else. Bottom line however, it doesn't break the integrity and simplicity of the base level library, even after Adirelle's additions to the API, the "High Level API" is very very thin.
I also think that its important that we do layer the api docs to describe basic usage with the high level functions and then describe lower level apis for advanced usages. Creating simple code is a Good Thing(tm). However, creating elegant code that appears simple but has a good level of depth and flexility is much better.
function Provider:Aquire(parent) cellObj.provider = self; return cellObj end
function Provider:Release(cellObj) end
function Provider:IterateCells() return pairs(self.cells) end
function CellObj:SetupCell(tooltip, value, font, justification, ...) return width, height end
function CellObj:Release() end -- Alias for self.provider:Release(self)
-- numCols defaults to 1
-- provider defaults to LabelProvider
-- ... gets passed in to the SetupCell at the end
function tipProto:SetCell(lineNum, colNum, value, font, justification[, numCols, provider, ...]) end
sample usage in the lib
if not cell then
cell = CreateCell(self, line, column)
line.cells[colNum] = cell
end
local fontString = cell.fontString
if font then
fontString:SetFontObject(font)
end
cell.justification = justification or cell.justification or column.justification
fontString:SetJustifyH(cell.justification)
fontString:SetText(tostring(value or " "))
fontString:Show()
local width, height = fontString:GetStringWidth(), fontString:GetStringHeight()
-- Set the cell size (required to have the fontString displayed)
cell:SetWidth(width)
cell:SetHeight(height)
cell:Show()
becomes something close to
if not cell then
cell = Provider:Aquire(self)
line.cells[colNum] = cell
end
local width, height = cell:SetupCell(self, value, font, justification, ...)
and
function tipProto:Clear()
for i, line in ipairs(self.lines) do
for j, cell in ipairs(line.cells) do
cell.fontString:SetText(nil)
cell.fontString:Hide()
cell:Hide()
ReleaseFrame(cell)
line.cells[j] = nil
end
becomes something close to
function tipProto:Clear()
for i, line in ipairs(self.lines) do
for j, cell in ipairs(line.cells) do
cell:Release() -- or cell.provider:Release(cell)
line.cells[j] = nil
end
There might be a few Getters we want to add to the provider api, as well as the cell api.
I just finished to drycode the API I given. The whole library is now 400 lines long, including CellProvider and colspan. Adapting it to Kaelten latest proposal wouldn't take too long, though I don't get the point of IterateCells.
And Kaelten, have anything against adding SetColumnLayout and AddColumn ?
@Tristanian: I know this is a wall of text but it is more a working document than the user documentation. In the latter, I would start with "getting started in two minute" then "Basic API reference" and finally "Advanced usages" ...
I just finished to drycode the API I given. The whole library is now 400 lines long, including CellProvider and colspan. Adapting it to Kaelten latest proposal wouldn't take too long, though I don't get the point of IterateCells.
@Tristanian: I know this is a wall of text but it is more a working document than the user documentation. In the latter, I would start with "getting started in two minute" then "Basic API reference" and finally "Advanced usages" ...
If for no other reason than people should never have to reach in to grab .cells
I have integrated Kaelten's API.
I have also clean up the argument checking, using error instead of assert.
Comments ?
Looks good so far, and I prefer error over assert as well. From our discussion last night I had slight reservations, but it looks like the level of abstraction has actually kept the code close to its previous size and after we've finalized it all - as Kaelten said - we can trim it down further. This also fits extremely well with my hopes to have this as a basis for a more advanced library (for those who need it) for things like clickable elements and provides a well-defined entry point for it.
I'm also beginning to wonder if I'll be contributing anything more than the initial code, since Adirelle is just blazing away at this. :)
We have got a first working version with CellProvider and colspans.
A rough test shows it works.
Now we need more testing, specifically :
1) does CellProvider works with non-text cells ?
2) is the tooltip and columns resizing ok with and without colspans ?
3) I feel there is still something wrong with the font parameter in CreateLine. Dunno why and when it happens.
If you find any bug you cannot easily fix, fill a ticket with the code to reproduce it.
Concerning 2) I already know there are some bug cases:
Suppose you create a tooltip of 3 lines containing 7 cells like this (number are content minimal width) :
[40] [60] [50]
[----200-----]
[60] [60] [60]
The current code will update column immediately after cell setting.
Line 1 : every column enlarge to match its content width.
Result (column width in brackets, cell spacing in parens): [40] (3) [60] (3) [50] - total width : 156
Line 2 : the right-most column is enlarged to fit the colspan content width. Its new width is the total content width minus all previous column widths and spacings : 200-40-60-3-3 = 94
Result : [40] (3) [60] (3) [94] - total width : 200
Line 3 : the first column is enlarged to 60, other columns are fine.
Result : [60] (3) [60] (3) [94] - total width : 214
The issue is that a width 200 is sufficient to effectively display the tooltip. We cannot know that before the tooltip is completly filled.Kaelten proposed to postpone colspan width calculation until the tooltip is shown. I'll try to do that.
Beside that, I'll normalize the error messages (there is no need to prefix with method name as the error should be issued for the calling code) and some variable names (replacing xxxProto by xxxPrototype everywhere and same with xxxMeta).
And last, while writing my test, I found that the :SetCell signature is quite cumbersome. I'm often passing empty fields for defaults and as they are at least 4 optional arguments, remembering their order is quite challenging. Couldn't we find a better way for passing them ? Are you against the ("argumentName", argumentValue) scheme ? By example :
Yes I am against trying to pass in named arguments. It's an awesome language feature. However, it is one we don't have.
There are a few options I can see:
1) provide additional, or more advanced higher level apis to cut down the need to use setcell directly
2) pass in things as a dictionary
3) leave it as it is
4) add code to detect what variables are what
Downsides
1) we have to be careful not to make the high level api unweildy
2) ewe, bad practice in general
3) doing this eliminates any chance of making it better
4) can be difficult but it is doable, adds complexity to setcell.
There are a few options I can see:
4) add code to detect what variables are what
Downsides
4) can be difficult but it is doable, adds complexity to setcell.
I like this one. If we look at the function signature:
function tipProto:SetCell(lineNum, colNum, value, font, justification, numCols, provider, ...) end
We have 4 options arguments :
- font is a Font object, a table with a IsObjectType method that checks font:IsObjectType ('Font'),
- justification is a string amongt "LEFT", "CENTER", "RIGHT",
- numCols is a number,
- provider is CellProvider, a table with an AcquireCell method,
The optional provider arguments are necessarily at the end. As the 4 arguments are of different types, we could just make them optional:
function tipProto:SetCell(lineNum, colNum, value[, font][, justification][, numCols][, provider, ...]) end
Not sure about the complexity it would add. I think this would look like this:
function tipProto:SetCell(lineNum, colNum, value, ...)
local font, justification, numCols, provider = self.regularFont, 'LEFT', 1, labelProvider
local i, arg = 1, ...
if type(arg) == 'table' and type(arg.IsObjectType) == 'function' and arg:IsObjectType('Font') then
i, font, arg = i+1, ...
end
if type(arg) == 'string' then
i, justification, arg = i+1, select(i, ...)
end
if type(arg) == 'number' then
i, numCols, arg = i+1, select(i, ...)
end
if type(arg) == 'table' and type(arg.AcquireCell) == 'function' then
i, provider = i+1, select(i, ...)
end
SetCell(self, lineNum, colNum, value, font, justification, numCols, provider, select(i, ...))
end
This thread would better be renamed "LibTooltip dev corner" and we should recreate an new "LibTooltip-1.0 official thread" when the first beta is released.
Indeed,b ut that is why we offer a way for you to use it without ever knowing.
I just mean that from a software engineer/developer perspective, context-sensitive arguments used to that extent makes a big mess in terms of readability. Anyone reading code that uses it will want to kill whoever wrote the code and the library.
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumActually there are two solutions for basic usage :
1) :AddLine is unchanged, so you could just do AddLine("label1", "label2", "label3")
2) the provider in :SetCell can be omitted, so you could do SetCell(1,1,"Label").
About the library size, Kaelten estimated we shouldn't get over 750 lines with both colspans and CellProvider (not counting comments). I'm inclined to think he is right.
Indeed it can, sorry but that wall of text wasn't actually more "readable" than Tablet documentation (no offense) and it took some time to sink in (perhaps I realized as well that I'm not as smart after all :p). Like I said, I'm not that much concerned about the size, since I do not foresee it will reach excessive or otherwise "unacceptable" amounts. I'm worried about the fact that we may be implementing something that will only be used rarely.
The reason that I want the cellprovider api, is that it can be extremely extensible. With a nice wrapper layer and enough custom providers you could recreate a dewdrop menu system, tablet like behaviors, or anything else. Bottom line however, it doesn't break the integrity and simplicity of the base level library, even after Adirelle's additions to the API, the "High Level API" is very very thin.
I also think that its important that we do layer the api docs to describe basic usage with the high level functions and then describe lower level apis for advanced usages. Creating simple code is a Good Thing(tm). However, creating elegant code that appears simple but has a good level of depth and flexility is much better.
sample usage in the lib
if not cell then cell = CreateCell(self, line, column) line.cells[colNum] = cell end local fontString = cell.fontString if font then fontString:SetFontObject(font) end cell.justification = justification or cell.justification or column.justification fontString:SetJustifyH(cell.justification) fontString:SetText(tostring(value or " ")) fontString:Show() local width, height = fontString:GetStringWidth(), fontString:GetStringHeight() -- Set the cell size (required to have the fontString displayed) cell:SetWidth(width) cell:SetHeight(height) cell:Show()becomes something close toif not cell then cell = Provider:Aquire(self) line.cells[colNum] = cell end local width, height = cell:SetupCell(self, value, font, justification, ...)andfunction tipProto:Clear() for i, line in ipairs(self.lines) do for j, cell in ipairs(line.cells) do cell.fontString:SetText(nil) cell.fontString:Hide() cell:Hide() ReleaseFrame(cell) line.cells[j] = nil endbecomes something close tofunction tipProto:Clear() for i, line in ipairs(self.lines) do for j, cell in ipairs(line.cells) do cell:Release() -- or cell.provider:Release(cell) line.cells[j] = nil endThere might be a few Getters we want to add to the provider api, as well as the cell api.
Thoughts?
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumAnd Kaelten, have anything against adding SetColumnLayout and AddColumn ?
@Tristanian: I know this is a wall of text but it is more a working document than the user documentation. In the latter, I would start with "getting started in two minute" then "Basic API reference" and finally "Advanced usages" ...
If for no other reason than people should never have to reach in to grab .cells
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumI have integrated Kaelten's API.
I have also clean up the argument checking, using error instead of assert.
Comments ?
-
View User Profile
-
View Posts
-
Send Message
Project AuthorLooks good so far, and I prefer error over assert as well. From our discussion last night I had slight reservations, but it looks like the level of abstraction has actually kept the code close to its previous size and after we've finalized it all - as Kaelten said - we can trim it down further. This also fits extremely well with my hopes to have this as a basis for a more advanced library (for those who need it) for things like clickable elements and provides a well-defined entry point for it.
I'm also beginning to wonder if I'll be contributing anything more than the initial code, since Adirelle is just blazing away at this. :)
-
View User Profile
-
View Posts
-
Send Message
Curse Premium-
View User Profile
-
View Posts
-
Send Message
Project AuthorI'm not expecting that to last :p
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumA rough test shows it works.
Now we need more testing, specifically :
1) does CellProvider works with non-text cells ?
2) is the tooltip and columns resizing ok with and without colspans ?
3) I feel there is still something wrong with the font parameter in CreateLine. Dunno why and when it happens.
If you find any bug you cannot easily fix, fill a ticket with the code to reproduce it.
Concerning 2) I already know there are some bug cases:
Suppose you create a tooltip of 3 lines containing 7 cells like this (number are content minimal width) :
The current code will update column immediately after cell setting.
Line 1 : every column enlarge to match its content width.
Result (column width in brackets, cell spacing in parens): [40] (3) [60] (3) [50] - total width : 156
Line 2 : the right-most column is enlarged to fit the colspan content width. Its new width is the total content width minus all previous column widths and spacings : 200-40-60-3-3 = 94
Result : [40] (3) [60] (3) [94] - total width : 200
Line 3 : the first column is enlarged to 60, other columns are fine.
Result : [60] (3) [60] (3) [94] - total width : 214
The issue is that a width 200 is sufficient to effectively display the tooltip. We cannot know that before the tooltip is completly filled.Kaelten proposed to postpone colspan width calculation until the tooltip is shown. I'll try to do that.
Beside that, I'll normalize the error messages (there is no need to prefix with method name as the error should be issued for the calling code) and some variable names (replacing xxxProto by xxxPrototype everywhere and same with xxxMeta).
And last, while writing my test, I found that the :SetCell signature is quite cumbersome. I'm often passing empty fields for defaults and as they are at least 4 optional arguments, remembering their order is quite challenging. Couldn't we find a better way for passing them ? Are you against the ("argumentName", argumentValue) scheme ? By example :
There are a few options I can see:
1) provide additional, or more advanced higher level apis to cut down the need to use setcell directly
2) pass in things as a dictionary
3) leave it as it is
4) add code to detect what variables are what
Downsides
1) we have to be careful not to make the high level api unweildy
2) ewe, bad practice in general
3) doing this eliminates any chance of making it better
4) can be difficult but it is doable, adds complexity to setcell.
I think this is one we need to talk on IRC about.
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumI like this one. If we look at the function signature:
We have 4 options arguments :
- font is a Font object, a table with a IsObjectType method that checks font:IsObjectType ('Font'),
- justification is a string amongt "LEFT", "CENTER", "RIGHT",
- numCols is a number,
- provider is CellProvider, a table with an AcquireCell method,
The optional provider arguments are necessarily at the end. As the 4 arguments are of different types, we could just make them optional:
Not sure about the complexity it would add. I think this would look like this:
function tipProto:SetCell(lineNum, colNum, value, ...) local font, justification, numCols, provider = self.regularFont, 'LEFT', 1, labelProvider local i, arg = 1, ... if type(arg) == 'table' and type(arg.IsObjectType) == 'function' and arg:IsObjectType('Font') then i, font, arg = i+1, ... end if type(arg) == 'string' then i, justification, arg = i+1, select(i, ...) end if type(arg) == 'number' then i, numCols, arg = i+1, select(i, ...) end if type(arg) == 'table' and type(arg.AcquireCell) == 'function' then i, provider = i+1, select(i, ...) end SetCell(self, lineNum, colNum, value, font, justification, numCols, provider, select(i, ...)) endIndeed,b ut that is why we offer a way for you to use it without ever knowing.
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumI just mean that from a software engineer/developer perspective, context-sensitive arguments used to that extent makes a big mess in terms of readability. Anyone reading code that uses it will want to kill whoever wrote the code and the library.