parasew/instiki

Re: Interweb links not working with [[!include web:pagename]]

davidtanzer opened this issue · 5 comments

It's close but not quite right. In the syntax [[!include web:pagename]] the current release is expecting web to be the full name of the web, not its address.

This is fixed by changing line 21 of lib/chunks/include.rb from:
@ref_web = Web.find_by_name(@web_name)
to:
@ref_web = Web.find_by_address(@web_name)

Also a smaller point: if web is an invalid name, an application error is generated, with an undefined method error in the log:

Processing WikiController#save (for 98.116.146.188 at 2020-06-12 19:48:00) [POST]
Parameters: {"_form_key"=>"9651e53ab855e95ed4f3e52ef823a3207118d4c1", "content"=>"[[!include notebook2:navigation]]", "new_name"=>"Sandbox", "author"=>"AnonymousCoward", "web"=>"act", "id"=>"Sandbox"}
Reading page 'Sandbox' from web 'act'
Page 'Sandbox' found
98.116.146.188
98.116.146.188
Reading page 'Sandbox' from web 'act'
Page 'Sandbox' found
HERE web notebook2 page navigation

NoMethodError (undefined method password' for nil:NilClass): lib/chunks/include.rb:24:in initialize'
lib/chunks/chunk.rb:53:in new' lib/chunks/chunk.rb:53:in block in apply_to'
lib/chunks/chunk.rb:52:in gsub!' lib/chunks/chunk.rb:52:in apply_to'
lib/wiki_content.rb:174:in build_chunks' lib/wiki_content.rb:160:in initialize'
lib/page_renderer.rb:140:in new' lib/page_renderer.rb:140:in render'
lib/page_renderer.rb:29:in display_content' app/models/page.rb:26:in revise'
app/models/wiki.rb:79:in revise_page' app/controllers/wiki_controller.rb:369:in save'
/usr/local/lib/ruby/gems/2.4.0/gems/passenger-5.1.2/src/ruby_supportlib/phusion_passenger/rack/thread_handler_extension.rb:97:in process_request' /usr/local/lib/ruby/gems/2.4.0/gems/passenger-5.1.2/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:160:in accept_and_process_next_request'
/usr/local/lib/ruby/gems/2.4.0/gems/passenger-5.1.2/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:113:in main_loop' /usr/local/lib/ruby/gems/2.4.0/gems/passenger-5.1.2/src/ruby_supportlib/phusion_passenger/request_handler.rb:416:in block (3 levels) in start_threads'
/usr/local/lib/ruby/gems/2.4.0/gems/passenger-5.1.2/src/ruby_supportlib/phusion_passenger/utils.rb:113:in `block in create_thread_and_abort_on_exception

BUT: the fix I just described causes this to fail [[!include navigation]]

That's the same way interweb WikiLinks work. Don't you think they should be consistent?

I just tested it, interweb WikiLinks work both using the display name and the address.   I had always been using the address, and assumed on the basis of consistency that the new include statement would use that as well.

If I were designing it from scratch I would use address consistently everywhere, as it feels more permanent, doesn't contain spaces and capitalization; it's an identifier.   It would be good to be able to tweak the display name of a web without breaking links to its pages.

But that would break backward compatibility.    If it were me I would still deprecate it, and use just address in the new syntax.

But we're getting into subjective judgement calls.   Whatever you decide the intended behavior to be is fine, I'll follow that.

Actually, either the name or address (with the name preferred) are allowed for interweb WikiLinks.

So we should do the same here. Added in 60db4d4. Tweaked in f6f5482.

P.S.: You can perfectly well comment on a closed issue and even (if you were the one who opened it) reopen the issue. No need to open a new one, merely to add a comment.