1617529 - Rename client/shared/components/AutoCompletePopup.js to SearchBoxAutocompletePopup

Status: RESOLVED FIXED

(DevTools :: Shared Components, task, P3)

Description

[Julian Descottes [:jdescottes]]

We have 2 autocomplete popup widgets:

• https://searchfox.org/mozilla-central/source/devtools/client/shared/components/AutoCompletePopup.js
• https://searchfox.org/mozilla-central/source/devtools/client/shared/autocomplete-popup.js

The first one is only used by the SearchBox shared react component, the second one is used directly by the inspector and the console. Both export a class with the same exact name "AutocompletePopup". We should rename one of them to make it less confusing. Since the first one is only used by SearchBox, renaming it to SearchBoxAutocompletePopup would probably be the most straightforward, but happy to consider other names :)

(spotted while reviewing Bug 1609942)

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Thanks for filing this Julian. Setting it up as a good-first-bug.

The file should be renamed using hg mv so we don't lose the history on it.

Comment 2

[Aggelos Gamvrinos [:agam]]

I quickly fixed this. Submitting a patch soon.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Thanks Aggelos, but as the tag indicates, it's a good first bug, and you already did your first bug :)
Let's keep that one for people whose this will really be their first bug.

Comment 4

[Aggelos Gamvrinos [:agam]]

Fair enough :)

Comment 5

[Aggelos Gamvrinos [:agam]]

Attachment: Bug 1617529 - Rename client/shared/components/AutoCompletePopup.js to SearchBoxAutocompletePopup r=jdescottes,nchevobbe

Comment 6

[mab]

I am new just trying to get started. Can any one help me to understand this bug, does we just need to rename the name of the function such that it does not export a class with same name like:
let SearchBoxAutocompletePopup = function AutocompletePopup(toolboxDoc, options = {}) {.....}
module.exports = SearchBoxAutocompletePopup;
or any think else.

Comment 7

[Julian Descottes [:jdescottes]]

(In reply to mab from comment #6)

I am new just trying to get started. Can any one help me to understand this bug, does we just need to rename the name of the function such that it does not export a class with same name like:
let SearchBoxAutocompletePopup = function AutocompletePopup(toolboxDoc, options = {}) {.....}
module.exports = SearchBoxAutocompletePopup;
or any think else.

Hi mab!

For this bug

• we need to first rename the file at https://searchfox.org/mozilla-central/source/devtools/client/shared/components/AutoCompletePopup.js to SearchBoxAutocompletePopup.js (use hg mv to do that)
• then in the file we need to rename both matches of AutocompletePopup to SearchBoxAutocompletePopup
• then we have to update the "index" for this file, rename AutocompletePopup to SearchBoxAutocompletePopup in https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/devtools/client/shared/components/moz.build#19 (! be careful the list needs to be alphabetically sorted, so SearchBoxAutocompletePopup should be between SearchBox.js and Sidebar.js
• finally in https://searchfox.org/mozilla-central/source/devtools/client/shared/components/SearchBox.js we have to rename AutocompletePopup to SearchBoxAutocompletePopup as well (https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/devtools/client/shared/components/SearchBox.js#17,19,243)

If you are able to build Firefox, simply doing a ./mach build should catch most of the mistakes.

Let us know if you have more questions.

Comment 8

[Julian Descottes [:jdescottes]]

Also the code you pasted let SearchBoxAutocompletePopup = function AutocompletePopup(toolboxDoc, options = {}) {.....} is from https://searchfox.org/mozilla-central/source/devtools/client/shared/autocomplete-popup.js . We don't want to rename this file, we want to rename the other one: https://searchfox.org/mozilla-central/source/devtools/client/shared/components/AutoCompletePopup.js . It can be confusing, so make sure you have the right file before renaming things :)

Comment 9

[tusharxdev]

Attachment: Bug 1617529 - Rename client/shared/components/AutoCompletePopup.js to SearchBoxAutocompletePopup. r=nchevobbe

Comment 10

[tusharxdev]

Attachment: Bug 1617529 - Rename client/shared/components/AutoCompletePopup.js to SearchBoxAutocompletePopup and update components importing it. r=nchevobbe

Comment 11

[tusharxdev]

Attachment: Bug 1617529 - Rename client/shared/components/AutoCompletePopup.js to SearchBoxAutocompletePopup. r=nchevobbe

Comment 12

[tusharxdev]

Attachment: Bug 1617529 - Rename client/shared/components/AutoCompletePopup.js to SearchBoxAutocompletePopup. r=nchevobbe

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6079218e54c
Rename client/shared/components/AutoCompletePopup.js to SearchBoxAutocompletePopup. r=nchevobbe

Comment 14

[Julian Descottes [:jdescottes]]

Thank you for fixing this bug tusharxdev!
It will make the code much easier to navigate.

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a6079218e54c