Grammars
Илья
I think separate grammar and parser is good idea (do you think so?)
I put our wiki grammar to:
Text/Markup/Wiki/Simple.pm
as
Text::Markup::Wiki::Simple
Does anyone have comments or objections?
Ilya
Vladivostok.pm
Carl Mäsak
> I think separate grammar and parser is good idea (do you think so?)
>
> I put our wiki grammar to:
> Text/Markup/Wiki/Simple.pm
> as
> Text::Markup::Wiki::Simple
> Does anyone have comments or objections?
Oops! I'm already working full speed on this in a separate (local)
branch. Sorry for duplicating work by not telling. :/
I've now pushed that branch. You can see it here.
<http://github.com/viklund/november/tree/markup>
It also adds tests for the minimal markup we're currently using. The
tests are what I'm working on today. Once I've written them, I plan to
merge back into master, but continue working on the branch, adding the
markup required to parse the p5w Main_Page.
<http://github.com/viklund/november/tree/master/p5w/data/modifications/1219443872>
Before that intermediate merge, I'll probably have to revert your
change, Илья -- although I fundamentally agree with it (and have done
the same in my branch).
// Carl
Carl Mäsak
> Oops! I'm already working full speed on this in a separate (local)
> branch. Sorry for duplicating work by not telling. :/
Here's me telling a bit more, to compensate. :)
I had an interesting experience separating out the markup parsing code
yesterday. Interesting to me, anyway, at my current (early) stage of
the learning curve towards becoming a real programmer.
It occurred when I wanted to move out format_html from Wiki.pm into
Wiki/Markup/Minimal.pm -- suddenly the method make_link($title), which
produces strings of the form '<a href="...">$title</a>' as part of the
eventual output, was not visible anymore, as it was still in Wiki.pm.
Big deal, so move that as well, right? Ah, but make_link actually does
different things depending on whether the page exists or not. To do
this, it calls methods on $.storage, an attribute of the Wiki class.
So make_link should remain in the Wiki class for this reason -- and
for other reasons, such as it being used in other parts of the wiki
except for the formatter.
Ok, so I wanted to move the make_link method, except I couldn't. To
me, this seemed like a good time to perform what some people call
"Dependency Injection", and which Wikipedia defines as "supplying an
external dependency to a software component". That was exactly what I
wanted to do. Preferably just send the method make_link along as a
parameter to the formatter somehow.
However... I never figured out how to do this. I don't know yet if I'm
missing something, if Rakudo isn't capable of it yet, or if I'm simply
trying to do the impossible. Some of my bug reports today have been
about this.
The basic problem is the following: what I want isn't just to send in
the method make_link as a parameter, but to send it in so that it is
then called as a sub, not as a method. I think I want something like
.assuming(self=>self). If anyone has any idea how to emulate this
today in Rakudo, let me know.
What I'm doing right now instead is keeping a $.wiki attribute in the
formatter, and having the formatter call make_link that way. Less than
ideal.
// Carl
Илья
> branch. Sorry for duplicating work by not telling. :/
> I've now pushed that branch. You can see it here.
Yes. My opinion Wiki:: is not good "namespace" for text markup.
I think about this, before I do my variant, it`s text markup. It`s not
very important, but i think Text::Markup::Wiki::Minimal is better.
> It also adds tests for the minimal markup we're currently using. The
> tests are what I'm working on today. Once I've written them, I plan to
> merge back into master, but continue working on the branch, adding the
> markup required to parse the p5w Main_Page.
> <http://github.com/viklund/november/tree/master/p5w/data/modifications/1219443872>
Good!
Carl Mäsak
>> I've now pushed that branch. You can see it here.
>
> Yes. My opinion Wiki:: is not good "namespace" for text markup.
> I think about this, before I do my variant, it`s text markup. It`s not
> very important, but i think Text::Markup::Wiki::Minimal is better.
You seem to have given more thought to good namespace naming than I
have. :) Actually, I think the whole topic deserves a full-blown
discussion at some point. There's some kind of a policy decision
pending about whether we are creating the namespaces "just for us", or
whether we are attempting to fit them into a more global, CPAN-like
pattern. Right now, we're doing something in between.
Илья, I'm changing the name to your suggestion in the markup branch.
However, we need to discuss naming in general.
// Carl
Илья
no Test.pm in markup branch, is not comfortable, mb merge from master?
Ilya
2008/9/14 Carl Mäsak <cma...@gmail.com>:
Илья
2008/9/14 Илья <for...@gmail.com>:
Илья
excuse my communication lag, I just need time to translate messages.
This is really bad to couple up Wiki engine and Wiki syntax parser
(and formater) each other.
Right now I see that way:
Controller (Wiki.pm) call View(T::M::W::Minimal) to parse content (by
::Grammar), process link list and look for non exists link by call
Model (Store::File). After that Controller call T::M::W::Minimal with
$/ and %param with something like {non_exists_link => ['foo_page',
'boo_page']}.
Put make_link into T::M::W::Minimal, with opts for set class. And wrap
for it to call with class => 'non_exist' if page is in
%param<non_exists_link>.
So... I can do that (I hope).
Ilya
2008/9/14 Carl Mäsak <cma...@gmail.com>:
>
Илья
that way Controller do application work, Model and View do not call
each other, and do not call Controller or it`s methods, just do it own
work separated. The MVC pattern -- mb how I understand it.
Carl Mäsak
> Carl,
> excuse my communication lag, I just need time to translate messages.
I understand. Thank you for your efforts.
> This is really bad to couple up Wiki engine and Wiki syntax parser
> (and formater) each other.
>
> Right now I see that way:
>
> Controller (Wiki.pm) call View(T::M::W::Minimal) to parse content (by
> ::Grammar), process link list and look for non exists link by call
> Model (Store::File). After that Controller call T::M::W::Minimal with
> $/ and %param with something like {non_exists_link => ['foo_page',
> 'boo_page']}.
>
> Put make_link into T::M::W::Minimal, with opts for set class. And wrap
> for it to call with class => 'non_exist' if page is in
> %param<non_exists_link>.
I disagree. Though I haven't studied Catalyst to any significant
degree, I think I understand the MVC idea well enough. To me, sending
the make_link method as a parameter provides exactly the decoupling
needed. (A variation on the theme is to provide only the page_exists
method from the Storage object. Amounts to the same thing.)
Right now, an inferior solution lingers in T::M::W::Minimal, wherein
the Wiki object is stored as an attribute. I have since discovered a
way to do this using real dependency injection, so I will replace this
code with that. To clarify: the current solution _is_ coupled, and I
know it is, and I know it's bad -- but it was the only way I could
solve the issue a few days ago.
I agree that it's really bad to couple the Wiki and the formatter. In
my view, the Dependency Injection trick avoids that. I'll let you know
when I've applied the change, so you can see for yourself.
// Carl
Илья
2008/9/17 Carl Mäsak <cma...@gmail.com>:
Carl Mäsak
> I agree that it's really bad to couple the Wiki and the formatter. In
> my view, the Dependency Injection trick avoids that. I'll let you know
> when I've applied the change, so you can see for yourself.
There. The dependency on Wiki from T::M::W::Minimal is now gone, and
things are as I intended them to be in the first place. Check it out
in either the master or the markup branch.
The call to format itself looks like this:
Text::Markup::Wiki::Minimal.new.format(
$.storage.read_page($page),
{ self.make_link($^page) }
)
The first parameter is the text, as usual. The second (optional)
parameter is a closure, called when an internal link is matched. Thus
1. The make_link method gets to stay in the Wiki class.
2. No code duplication.
3. No deps from formatting code on Wiki.
4. The format method can even be used as before. If no second param is
sent, internal link syntax is simply ignored. See
t/markup/minimal/06-links.t.
I'd say that is acceptable, yes?
// Carl
Carl Mäsak
<http://masak.org/carl/dependency-injection.png>
// Carl
Илья
8) Thank you very much!
( It`s looking grate. Can I print one and put on the wall in our office? :) )
I have some ideas about that implementation, I write it latter,
because I need time to certain about I fully understand it.
Ilya
Carl Mäsak
>> <http://masak.org/carl/dependency-injection.png>
> 8) Thank you very much!
> ( It`s looking grate. Can I print one and put on the wall in our office? :) )
Be my guest. If you wait till this afternoon, I will have had time to
put a cc-license on it, too.
> I have some ideas about that implementation, I write it latter,
> because I need time to certain about I fully understand it.
I'm glad the picture can be of use.
// Carl
Илья
> I disagree. Though I haven't studied Catalyst to any significant
> degree, I think I understand the MVC idea well enough. To me, sending
> the make_link method as a parameter provides exactly the decoupling
> needed. (A variation on the theme is to provide only the page_exists
> method from the Storage object. Amounts to the same thing.)
I think provide page_exists is better way than make_link. Right now
our ::Minimal do not able to make link by it self, and need make_limk
always :( I will try to change that in markup brunch. For example: I
try to make [[link title]] work, but that depends to make_link
realization.
Ilya
Carl Mäsak
>> I disagree. Though I haven't studied Catalyst to any significant
>> degree, I think I understand the MVC idea well enough. To me, sending
>> the make_link method as a parameter provides exactly the decoupling
>> needed. (A variation on the theme is to provide only the page_exists
>> method from the Storage object. Amounts to the same thing.)
>
> I think provide page_exists is better way than make_link. Right now
> our ::Minimal do not able to make link by it self, and need make_limk
> always :(
Well, technically not always; only when the input string contains the
link markup. The make_link parameter is optional, and if it isn't
provided, things work fine.
> I will try to change that in markup brunch. For example: I
> try to make [[link title]] work, but that depends to make_link
> realization.
That's a nice feature to have, but I see why it would be a problem
with the current setup. Though adding things to ::Minimal is not a
priority right now, the question is still interesting because we will
run into the same problem with ::MediaWiki.
Thank you for stopping by on IRC, by the way. :) We could continue the
discussion there, if you want.
// Carl