Docs: Replace "off" with false [ci skip]

[seanpdoyle]

Detail

Replace a reference to a default "off" value in the ActiveRecord::NestedAttributes module with the actual default value: false.

[jonathanhefner]

Nitpick: I think "false" here should not use monospace because it is not referring to the false singleton. In this case, the default is actually (probably) nil, but that does not matter to the user, so we can say "false" to contrast with "If true".

[seanpdoyle]

In this case, the default is actually (probably) nil

@jonathanhefner I might be misreading the implementation, but the default options declare allow_destroy: false:

rails/activerecord/lib/active_record/nested_attributes.rb

Lines 351 to 355 in 8d778e0

	def accepts_nested_attributes_for(*attr_names)
	options = { allow_destroy: false, update_only: false }
	options.update(attr_names.extract_options!)
	options.assert_valid_keys(:allow_destroy, :reject_if, :limit, :update_only)
	options[:reject_if] = REJECT_ALL_BLANK_PROC if options[:reject_if] == :all_blank

I'm happy to remove the wrapping + characters, but I want to make sure that I understand the implications first.

[p8]

+false+ is inline with the description of :update_only: By default the +:update_only+ option is +false+.
Maybe also change If true, destroys to If +true+, destroys ? ...although I don't have a strong opinion here.

[p8]

Reading the link from @jonathanhefner I agree with his suggestion.

Do not document singletons unless absolutely necessary (as) that... allows refactors, and the code does not need to rely on the exact values returned by methods being called in the implementation.

[jonathanhefner]

You are correct about it being false; I didn't actually look at the implementation and just assumed it was nil. But the point is that it doesn't matter to the user — indeed, the user will never see or interact with this value — and we could change the implementation to use nil without the user knowing.

In other words, the default is "falsy", just like the enabling value merely needs to be "truthy", not true. But we avoid those words in Rails documentation in favor of "false" and "true" without monospace formatting.

[seanpdoyle]

@jonathanhefner thank you for sharing the link to https://guides.rubyonrails.org/v7.1/api_documentation_guidelines.html#booleans, I had no idea there were style guides for documentation! I'll read up on the rest of what's provided there.

In the meantime, with the Boolean documentation in mind, is this diff valuable at all? Is "false" an improvement over "off"? Should I replace +false+ with false, or would it be better to close this without change?

[jonathanhefner]

We should not use +false+. I don't have a strong opinion on "false" vs "off", but if you think it reads more clearly, then I'm 👍.

[p8]

If we are going to change this, I think we should probably also change the following to use false and true?:

https://github.com/rails/rails/blob/3e839e4457f535ea80cbcc797eb969392210dbe0/activerecord/lib/active_record/nested_attributes.rb#L335

https://github.com/rails/rails/blob/3e839e4457f535ea80cbcc797eb969392210dbe0/activerecord/lib/active_record/nested_attributes.rb#L339

[jonathanhefner]

Thank you, @seanpdoyle! 0️⃣ 1️⃣ 0️⃣ And thank you, @p8! 👋

[zzak]

I'm not sure this applies here, but reminded me of the guide for booleans.

[p8]

@zzak Jonathan mentioned that as well above: #49936 (comment)