1025691 - Use C Macros for eAutcompleteField* enums

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[[ex-Mozilla] Agi Sferro | :agi]

From Bug 1009935 Comment 21. We could use C macros to build eAutocompleteField* instead to have the same field in two places. I'll post a small patch for it in this bug.

Comment 1

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Use Macro for eAutocompleteField* enums

Olli, could you take a look? Feel free to invalidate the bug if you think it's not worth it. 

Thanks!

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I'll look at this tomorrow.

Comment 3

[[ex-Mozilla] Agi Sferro | :agi]

Thanks! Try: https://tbpl.mozilla.org/?tree=Try&rev=1e298f703866

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8440448 [details] [diff] [review]
Use Macro for eAutocompleteField* enums

>+ * The first argument to AUTOCOMPLETE_* macros will be concatenated
>+ *   to form the C++ name of the attribute
extra spaces before  'to'. But the comment seem to be a bit wrong anyway.
It is up to the user to do something with the name.
Not sure what would be right... something like
"The first argument of AUTOCOMPLETE_* macros is the identifier for the token."
>+ * The second argument is the string value of the attribute
hmm, "attribute" or should it be token or some such.

>+// Contact category types
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(TEL, "tel")
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(TEL_COUTRY_CODE, "tel-country-code")
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(TEL_NATIONAL, "tel-national")
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(TEL_AREA_CODE, "tel-area-code")
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(TEL_LOCAL, "tel-local")
>+// AUTOCOMPLETE_CONTACT_FIELD_NAME(IMPP, "impp")
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(TEL_LOCAL_PREFIX, "tel-local-prefix")
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(TEL_LOCAL_SUFFIX, "tel-local-suffix")
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(TEL_EXTENSION, "tel-extension")
>+AUTOCOMPLETE_CONTACT_FIELD_NAME(EMAIL, "email")
>+// AUTOCOMPLETE_CONTACT_FIELD_NAME(IMPP, "impp")

You have IMPP twice. Remove the first one.

>-  eAutocompleteFieldName_last, // Dummy to check table sizes
>+  #define AUTOCOMPLETE_FIELD_NAME(name_, value_) \
>+          eAutocompleteFieldName_##name_,
I think all the #defines should use just 2 spaces for indentation in the next line.
(coding style probably doesn't say anything about this kind of rarely used thing)


r- just because I'd like to see the final patch before it is pushed to tree.

Comment 5

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1025483 - Use Macro for AutocompleteField* enums

Ok, I addressed your comments and put back the { 0 } at the end of EnumTables so we actually pass tests now.

Try: https://tbpl.mozilla.org/?tree=Try&rev=91303e322e5c

Thank you!

Comment 6

[:Gijs (he/him)]

Please use the right bug number in future.

https://hg.mozilla.org/integration/mozilla-inbound/rev/83d14fd3d739
https://hg.mozilla.org/integration/mozilla-inbound/rev/b563ec69fa02
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d0abd034712

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9d0abd034712