Remove *_without_bootstrap methods (BootstrapForm::Aliasing)

[GBH]

BootstrapForm::Aliasing needs to go. There's a reason why Rails ditched this meta-programming hackiness.

The thing is, you can live without it. Observe:

<%= bootstrap_form_for @user do |form| %>
  <%= form.text_field :misc %>
  <%= fields_for @user do |unformatted_form| %>
    <%= unformatted_form.text_field :misc %>
  <% end %>
<% end %>

All you need to do is wrap fields you don't want to be formatted in fields_for (fields_with behave the same). Resulting HTML currently looks like this:

<form>
  <div class="form-group">
    <label for="user_misc">
      Misc
    </label>
    <input class="form-control" type="text" name="user[misc]" id="user_misc" />
  </div>
  <input type="text" name="user[misc]" id="user_misc" />
</form>

This is a documented Rails thing and anybody who ever worked with forms should be aware what fields_forcan do. Also you can pass your own :builder option into it as well.

This will remove a lot of needless code.

For convenience sake I'm not against introducing option into form helper to bypass bootstrap wrappers as per @jeffblake as it's literally a single line addition.

define_method(method_name) do |name, options = {}|
  return super(name, options) if options[:super]
  form_group_builder(name, options) do
    prepend_and_append_input(options) do
      super name, options
    end
  end
end

[lcreid]

I'd really like to take a look at this idea. But at the moment, I'll confess I'm struggling a little to keep up. Therefore, I'd like to hold off on some of these improvements until we merge the form_with code #369.

[mattbrictson]

I agree a nice chunk of code would get removed. However I am not 100% convinced that this can be removed without consequence. I would like to circle back to this after 4.0.0 is released. I consider v4 compatibility to be a higher priority than API cleanliness.

[GBH]

@mattbrictson did you see my PR #387 ? I literally removed it without consequence in like 5 minutes.

[mattbrictson]

Yes, I've seen #387. I am concerned with the consequences to end users, not to our test suite. I make heavy use of _without_bootstrap to build custom controls in my apps (as I am sure other developers do as well) and I need more time to understand how swapping this with fields_for will impact all of these use cases.

[mattbrictson]

I'd accept a PR that changes the README to explain how to use the fields_for approach and makes it the recommendation for new users.

[lcreid]

It sounds to me like this change might break users' existing code. If that's the case, I'd ask if there's any way to support both the old way and the new way (sorry, I haven't had time to look at this in detail, so I don't even now if that's possible)? I think we need to consider what are the benefits to our users for any change that breaks their existing code.

[GBH]

@mattbrictson I can add entry in the readme. Any thoughts about the option that immediately calls super? That way you don't need to worry about fields_for. I just don't want to spend time if PR won't get merged.

@lcreid With major version bump it's acceptable (and expected) to have breaking changes.

[lcreid]

What is the benefit to our end users that they receive in return for having to change their code, which up to now has been working fine?

[GBH]

Knowing that library they use is not bloated, free of bugs and trivial to maintain to ensure future compatibility.

[mattbrictson]

Yes, there is always a balance to be struck. Too much cruft, and the project collapses under its own weight. Too frequent/jarring API changes, and users get frustrated and go elsewhere (or never upgrade). Both extremes are bad. I am sure we will come back to this at some point, but for now I am suggesting to de-prioritize it.

[lcreid]

I second that. And I want to be clear that I'm not saying @gbh 's PR is a bad idea. Let's get version 4.0 out there and then come back and look at this PR.

[GBH]

If this doesn't get removed for 4.0.0, it will never going to be removed 🤷‍♂️

[mattbrictson]

@GBH part of ensuring future maintenance of a project is fostering a healthy community of contributors. To achieve that I am doing my best to make this these discussion forums a friendly and welcoming environment even when hard decisions have to be made. I know you don't have much confidence in the maintainers of this project, but I would appreciate a little less negativity and cynicism here. Thanks :heart:

[GBH]

@mattbrictson you're reading too much into this. All I'm saying that any major changes need to be done before 4.0.0 release because after that you'll have a very valid reason not to change API.