Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attribute names exported to external file #1862

Merged
merged 8 commits into from
Sep 27, 2021
Merged

Attribute names exported to external file #1862

merged 8 commits into from
Sep 27, 2021

Conversation

andrey-helldar
Copy link
Member

I think this approach will be more correct.

I must say right away that, at the moment, automatic key processing does not work as we need. It stores the Foo and Bar keys in the validation files, and also creates these keys in the Ba file.

I am working on extracting the code for checking files and compiling documentation into an external project, and in it I will take this feature into account.

@luisprmat
Copy link
Member

It seems a bit more complicated for the contributor who helps us to translate, since he must look for the attributes in separate files and not as they come in Laravel by default.

I'm not saying that I disagree but at least we must document it very well.
Regarding this I think that with all the changes the documentation requires an urgent revision!

@andrey-helldar
Copy link
Member Author

On the one hand, you're right.

But on the other hand, the Laravel project does not contain translation of attributes. In addition, in our project it is specified only in the validation.php file.

However, an application using inline translations can also use attribute names for other purposes, not only for validation.

This means that localization managers will not be able to easily retrieve the values ​​they need and will have to use a more complex method for retrieving the values.

In addition, the current state of the attributes is difficult to maintain due to the fact that the keys are in the same file.

That is why I propose to move them to an external file both for the convenience of translations and for ease of use.

As for the documentation, we can specify that the translation keys will be in a third-party file, and the translation status does not require updating - the script will automatically generate everything.

@andrey-helldar andrey-helldar changed the title Attribute names exported to external file WIP: Attribute names exported to external file Sep 25, 2021
@andrey-helldar andrey-helldar changed the title WIP: Attribute names exported to external file Attribute names exported to external file Sep 25, 2021
@andrey-helldar andrey-helldar changed the title Attribute names exported to external file WIP: Attribute names exported to external file Sep 25, 2021
@andrey-helldar andrey-helldar changed the title WIP: Attribute names exported to external file Attribute names exported to external file Sep 25, 2021
@masterix21
Copy link
Member

I don't know if this is the right way: seemingly, it is better, but it could finally seem a bit confusing. I vote to keep our translations as more as near to the original files.

@andrey-helldar
Copy link
Member Author

Yes, but there are no translations of the attribute names in the original files.

Alternatively, you can move the translation of the attributes into an external package. For example, like here.

@masterix21
Copy link
Member

Yes, I prefer the external package.

@andrey-helldar
Copy link
Member Author

andrey-helldar commented Sep 25, 2021

@Laravel-Lang/laravel-lang, what do you say?

Yesterday I wanted to create a repository for attribute names, but then this idea seemed dubious to me.
It's easy to create a repository, there is a template for that.

@masterix21 also agrees to move these keys to a new repository.

Who else will support this idea?

@luisprmat
Copy link
Member

It seems to me that maintaining translations should be a responsibility of this package.

Therefore, although the attributes do not appear by default in the laravel validation.php file, if we do not translate them, we will see incomplete translations mixed with English. So if we remove them I think that we would be removing functionality from this package whose objective is to keep all the translations used by the Laravel framework and its other packages up to date.

@andrey-helldar
Copy link
Member Author

It also seems to me that these translations should be kept in this package.

But storing them in file validation.php seems like a bad idea to me, because projects that use inline translations may also need translations of attribute names.

That is why I moved them to an external files.

Moreover, yesterday I created a repository for these keys, but after half an hour I deleted it, as it looked like a terrible overhead.

@masterix21
Copy link
Member

On second thought, a separated repository might be a suicide too.

So, ok for the same repository but in a dedicated file.

@andrey-helldar
Copy link
Member Author

In this case, this PR is ready.

Copy link
Member

@luisprmat luisprmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation-inline.php was supposed to be NOT to handle custom attributes but generic attributes, but I understand your point of view.
So for me there is no problem that the attributes are in a file dedicated by language since the translator manager does the merge again!.

@andrey-helldar
Copy link
Member Author

andrey-helldar commented Sep 26, 2021

For example, my manager does not merge these files, but I use the attribute names along with inline translations.

Case:

<label for="first_name">
  {{ __('validation.attributes.first_name') }}
</label>

<input type="text" id="first_name" />

@error('first_name')
  <div class="alert">
    {{ $message }}
  </div>
@enderror

@caouecs caouecs merged commit 5af48f7 into Laravel-Lang:master Sep 27, 2021
@caouecs
Copy link
Member

caouecs commented Sep 27, 2021

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants