Skip to content

"buglist.cgi": Refactor semantical non-sense on determining "$order".#156

Open
MasterInQuestion wants to merge 3 commits intobugzilla:5.2from
MasterInQuestion:patch-2
Open

"buglist.cgi": Refactor semantical non-sense on determining "$order".#156
MasterInQuestion wants to merge 3 commits intobugzilla:5.2from
MasterInQuestion:patch-2

Conversation

@MasterInQuestion
Copy link
Copy Markdown

@MasterInQuestion MasterInQuestion commented Jan 1, 2024

    References:
    https://perldoc.perl.org/functions/last
    https://perldoc.perl.org/perlsyn#Foreach-Loops
    https://perldoc.perl.org/perlsyn#Basic-BLOCKs

    Nor is "$order" an iterable array.

    Also added some minor adaptation on RegEx to allow plausible variations.

    For RegEx help: https://regex101.com/

	References:
	https://perldoc.perl.org/functions/last
	https://perldoc.perl.org/perlsyn#Foreach-Loops
	https://perldoc.perl.org/perlsyn#Basic-BLOCKs

	Nor is "$order" an iterable array.

	Also added some minor adaptation on RegEx to allow plausible variations.
	And optimized RegEx quantifier's usage (of the "split") for performance.

	For RegEx help: https://regex101.com/

$order =~ /^(?:(?:[Ll]ast[ _]?)?[Cc]hanged?(?:[ _]?[Dd]ate)?|lc)$/ ?
( "changeddate", "bug_status", "priority", "assigned_to", "bug_id" ) :

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Where are these modifications coming from? For instance, "Last Changed" becomes /^(?:(?:[Ll]ast[ _]?)?[Cc]hanged?(?:[ _]?[Dd]ate)?|lc)$/ with no reason.

This makes the code unreadable, prone to errors, and I see no reason to change the list of accepted input.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

    "to allow plausible variations", for extensibility.
    So "order" may be specified as other variants: e.g. "order=lc" in request URL.

    Readability-wise, to represent a string group (and related operation):
    Such RegEx is already the optimal form: no other possibilities possible.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That is not true. //i would be much more readable than [Ll], [Cc] and co. Also, I see no valid reason to accept several different inputs for the same thing. In all cases, this should be discussed in a bugzilla bug first.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

    Is "lAsT CHaNgED" expected..?
    Writing "order=Last%20Changed" in URL is a pain...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Expected ? No. But being case-insensitive could make sense.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

    Of course.
    [ Quote MasterInQuestion @ CE 2023-12-01 04:44:44 UTC:
https://github.com/curl/curl/discussions/12397#discussioncomment-7674753
    CLI comparing typical programming languages: ease of input would be emphasized in addition.
    That's why PowerShell style `Get-Help Out-String -Full` needs shortcut. ]

    See also: https://github.com/MasterInQuestion/talk/discussions/15#discussion-6022492
    "Interactive vs. Canonical"

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.

2 participants