Skip to content

prompt: case-insensitive sorting for subscriptions #4969

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions cli/azd/pkg/prompt/prompter.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"os"
"slices"
"strconv"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/MakeNowJust/heredoc/v2"
Expand All @@ -22,6 +21,7 @@ import (
"github.com/azure/azure-dev/cli/azd/pkg/cloud"
"github.com/azure/azure-dev/cli/azd/pkg/environment"
"github.com/azure/azure-dev/cli/azd/pkg/input"
"github.com/azure/azure-dev/cli/azd/pkg/stringutil"
)

type LocationFilterPredicate func(loc account.Location) bool
Expand Down Expand Up @@ -157,7 +157,7 @@ func (p *DefaultPrompter) PromptResourceGroupFrom(
}

slices.SortFunc(groups, func(a, b *azapi.Resource) int {
return strings.Compare(a.Name, b.Name)
return stringutil.CompareLower(a.Name, b.Name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vhvb1989 , @JeffreyCA made a really good point on the linked issue that resource groups should behave similarly.

I suspect it goes beyond just these resources, so I added a new comparison function that avoids string allocations (previous implementation used strings.ToLower which creates a new string copy per each element iteration), if we wanted to do this more generally.

However, I'm truly wondering if we're down the right track here. There are many different Azure resources that will likely encounter the same problem, and I'm not sure if we'll want to apply this rule consistently.

Does the case-insensitive comparison truly feel any better? Presentation-wise it can get look a little messy at times with mixed capitalization. Curious what everyone thinks here -- My position shifted to slightly against this after looking at it more.

Copy link
Member

Choose a reason for hiding this comment

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

As long as the output is deterministic, I don't have any strong opinion about the sorting details.

Similar prompts are: Key vault Accounts and Secrets (or maybe not due to naming-rules for key vault), AI Models listing, AI Projects listing, etc.

Copy link
Contributor

@wbreza wbreza Mar 26, 2025

Choose a reason for hiding this comment

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

I wouldn't have any objections with just an easy normalized to lower based sorting instead of something more complex considering any lists we show are relatively small (< 100 items).

Copy link
Contributor

Choose a reason for hiding this comment

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

To me yes it feels a little more natural than listing names starting with A-Z followed by a-z. I found this resource naming doc, which I don't know if it's helpful. Most names seem to be case-sensitive, but it varies quite a bit whether uppercase letters are permitted in the name

})

canCreateNeResourceGroup := !options.DisableCreateNew
Expand Down Expand Up @@ -218,6 +218,10 @@ func (p *DefaultPrompter) getSubscriptionOptions(ctx context.Context) ([]string,
return nil, nil, nil, fmt.Errorf("listing accounts: %w", err)
}

slices.SortFunc(subscriptionInfos, func(a, b account.Subscription) int {
return stringutil.CompareLower(a.Name, b.Name)
})

// The default value is based on AZURE_SUBSCRIPTION_ID, falling back to whatever default subscription in
// set in azd's config.
defaultSubscriptionId := os.Getenv(environment.SubscriptionIdEnvVarName)
Expand Down
43 changes: 43 additions & 0 deletions cli/azd/pkg/stringutil/stringutil.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package stringutil

import (
"unicode"
"unicode/utf8"
)

// CompareLower returns an integer comparing two strings in a case-insensitive manner.
// The result will be 0 if a == b, -1 if a < b, and +1 if a > b.
//
// This comparison does not obey locale-specific rules.
// To compare strings based on case-insensitive, locale-specific ordering, see [golang.org/x/text/collate].
func CompareLower(a, b string) int {
for {
rb, nb := utf8.DecodeRuneInString(b)
if nb == 0 {
// len(a) > len(b), a > b.
return 1
}

ra, na := utf8.DecodeRuneInString(a)
if na == 0 {
// len(b) > len(a), b > a.
return -1
}

rb = unicode.ToLower(rb)
ra = unicode.ToLower(ra)

if ra > rb {
return 1
} else if ra < rb {
return -1
}

// Trim slices to the next rune.
a = a[na:]
b = b[nb:]
}
}
47 changes: 47 additions & 0 deletions cli/azd/pkg/stringutil/stringutil_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package stringutil

import (
"slices"
"testing"
)

func TestCompareLowerSort(t *testing.T) {
unordered := []string{
"Zebra",
"apple",
"applesauce",
"Banana",
"CHERRY",
"date",
"",
"Apple",
"café",
"cafe",
"APPLE",
}

expected := []string{
"",
"apple",
"Apple",
"APPLE",
"applesauce",
"Banana",
"cafe",
"café",
"CHERRY",
"date",
"Zebra",
}

slices.SortFunc(unordered, func(a, b string) int {
return CompareLower(a, b)
})

if !slices.Equal(unordered, expected) {
t.Errorf("incorrect sort order:\ngot: %q\nwant: %q", unordered, expected)
}
}
Loading