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

Removing dependency on args being in alphabetical order. #507

Merged
merged 2 commits into from
May 17, 2022

Conversation

ElonVolo
Copy link
Collaborator

Previously, the arg parser would display the arguments in the help section in alphabetical order. If you run jscodeshift -h you've notice all the arguments are displayed in alphabetical order. The algorithm that prints out the help page actually relies on them being in alphabetical order. The problem with this approach is that if there's a command that should be included in a category of arguments that appears in the middle of the page, but the new argument starts with a "z", then the new argument will appear in the last lines on the help page, and not in the middle where it's supposed to be.

Since we're need to move in the direction of grouping arguments by functionality independent of names (like for upcoming --gitignore flag), we now have to come up with a quick refactoring that allows arguments to be displayed in a specified order regardless of what alphabetical order the arguments themselves occupy.

Previously, arg parser would display the arguments in the help section in alphabetical order. Since we're now moving in the direction of grouping arguments by functionality independent of names (like for upcoming --gitignore flag), we now have to come up with a quick refactoring that allows arguments to be displayed in a specified order regardless of what alphabetical order the arguments themselves occupy.
@ElonVolo
Copy link
Collaborator Author

@Daniel15

Comment on lines 51 to 61
.sort((a,b) => {
if (a.display_index > b.display_index) {
return 1;
} else if (a.display_index < b.display_index) {
return -1;
} else if (a.display_index === b.display_index) {
return 0;
} else {
return -1;
}
});
Copy link
Member

@Daniel15 Daniel15 May 10, 2022

Choose a reason for hiding this comment

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

Could you just do this:

.sort((a,b) => a.display_index - b.display_index)

It's the common way to sort based on a numeric value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed

@Daniel15
Copy link
Member

like for upcoming --gitignore flag

Can we call that something more generic so that it could handle other ignore files (like .hgignore) too?

@Daniel15 Daniel15 merged commit adef04b into facebook:main May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants