Fix slime generator

[juanvico]

Fixes #157

[cpjolicoeur]

Thanks for this. I'm not a slime user personally, (as you can tell since I wasn't even aware the tool was actually called "slime" not "slim."

Let me look this over and see if there is a way we can DRY up some of the naming and duplication of the non-template specific files and code.

I'll try to get to this sometime in the next few days.

[cpjolicoeur]

Actually, looking at the phoenix-slime docs, it appears their generators output files with the .slim extension, not .slime so we probably should adhere to that here as well? Again, I'm not a slim/slime user so looking for input/feedback here.

[cpjolicoeur]

OK, I've studied the phoenix-slime generator code a bit more and see why & how they are using the existing Phoenix Mix tasks and generators, and why their default templates have the .eex extension which is then converted into a .slime extension.

[juanvico]

OK, I've studied the phoenix-slime generator code a bit more and see why & how they are using the existing Phoenix Mix tasks and generators, and why their default templates have the .eex extension which is then converted into a .slime extension.

It's exactly as you said. We think there is a bug or at least some enhancement to be done on the phoenix_slime package, but we thought it'd be easier and more straight forward to fix it just here.

The issue is actually in this line where the name should also have its suffix replaced. But this cascades into a lot more changes.

Anyway, we will fix the other comments regarding backward compatibility and DRYness so we can get this PR merged if it's alright with you.

[cpjolicoeur]

So I think that line is actually fine. The tuple the slime generator returns there is intended to mimic what the native phx.gen.html generator code is looking for and has a shape of

{template_type, template_file_name, generated_file_output_path}

For example:

[
  {:eex, "controller.ex",       Path.join([web_prefix, "controllers", web_path, "#{schema.singular}_controller.ex"])},
  {:eex, "edit.html.eex",       Path.join([web_prefix, "templates", web_path, schema.singular, "edit.html.eex"])},
  {:eex, "form.html.eex",       Path.join([web_prefix, "templates", web_path, schema.singular, "form.html.eex"])},
  {:eex, "index.html.eex",      Path.join([web_prefix, "templates", web_path, schema.singular, "index.html.eex"])},
  {:eex, "new.html.eex",        Path.join([web_prefix, "templates", web_path, schema.singular, "new.html.eex"])},
  {:eex, "show.html.eex",       Path.join([web_prefix, "templates", web_path, schema.singular, "show.html.eex"])},
  {:eex, "view.ex",             Path.join([web_prefix, "views", web_path, "#{schema.singular}_view.ex"])},
  {:eex, "controller_test.exs", Path.join([test_prefix, "controllers", web_path, "#{schema.singular}_controller_test.exs"])},
]

So the slime task first asks the native phx tasks for the list of files it needs to process:

 Mix.Tasks.Phx.Gen.Html.files_to_be_generated(context)

Which returns an array of the tuples listed above. The slime generator then just changes the output path file extension to be slim instead of eex.

The reason the slime package uses eex templates extensions in their templates is so they can leverage using the phx.gen.html code which parses the file through the EEx.eval_file. When calling Mix.Phoenix.copy_from, that task only looks for and handles files with a :text, :eex or :new_eex type

  case format do
    :text -> Mix.Generator.create_file(target, File.read!(source))
    :eex  -> Mix.Generator.create_file(target, EEx.eval_file(source, binding))
    :new_eex ->
      if File.exists?(target) do
        :ok
      else
        Mix.Generator.create_file(target, EEx.eval_file(source, binding))
      end
  end

Anyways, all that being said, I understand why, when Torch copies it's slime files over, they need to have the eex extension for the phx.gen.html.slime task to work properly and find the files. It's that task (phx.gen.html.slime) that will ultimately generate the files with an extension of .slim as the output.

Thanks for the response and help on this though. I'd love to get some of those other changes finished, and then we can get this PR merged and ship a new release out.

[cpjolicoeur]

@juanvico Any movement on this? Do you want/need me to jump in and help get this PR merged or are you both still working on the suggested changes?

[cpjolicoeur]

@juanvico Just wanted to ping you one more time on this before we jumped in and tried to finish up this work ourselves.

[brunvez]

@juanvico Just wanted to ping you one more time on this before we jumped in and tried to finish up this work ourselves.

@cpjolicoeur sorry for the late reply, it's been a busy couple of weeks. Do you have any progress towards this? I can get the requested changes by Tomorrow or Wednesday if you don't.

[cpjolicoeur]

@brunvez I haven't started working on this yet, no. I was waiting to hear back from you, otherwise I was going to dive into it sometime this week.

[brunvez]

Cool, then if you don't mind I'll make the suggested changes and re-request your review

[brunvez]

@cpjolicoeur updated. I added a common folder for common files, converted the format to be compatible, and updated the slime generator to be Phoenix 1.5 compatible.

Please let me know of any feedback you have. I wanted to add some tests but couldn't find any mirroring the behavior on eex

[juanvico]

@cpjolicoeur updated. I added a common folder for common files, converted the format to be compatible, and updated the slime generator to be Phoenix 1.5 compatible.

Please let me know of any feedback you have. I wanted to add some tests but couldn't find any mirroring the behavior on eex

Thanks for the help @brunvez!

[cpjolicoeur]

Thanks @brunvez I'll check this out today or tomorrow and look to get it in and released.

[cpjolicoeur]

@brunvez and @juanvico Thanks for the work on this. Version 3.3.0 was released today with these changes.