Help text includes command descriptions, platforms, and output formats#1005
Help text includes command descriptions, platforms, and output formats#1005freakboy3742 merged 7 commits intobeeware:mainfrom
Conversation
|
New help text examples: |
|
A lot of room for opinion here....sooo... There's (at least) two levels of review: presentation and code. I'm mostly interested in presentation right now....some of the code is dubious; I'll call these out in code comments. (There's probably also a need for some new tests.) As for the presentation:
|
| "Platforms:\n" | ||
| f" {', '.join([p.title() if p.islower() else p for p in platforms])}\n" |
There was a problem hiding this comment.
I think the list of Platforms look better when they are capitalized. However, this avoids capitalizing words like macOS and iOS.
There was a problem hiding this comment.
I think we have a big decision to make here.
The command line interface is forgiving about case, but the capitalisation matters from the perspective of configuration files - you actually need to use tool.briefcase.app.myapp.windows.VisualStudio. There's a "preferred" capitalisation for every platform and backend... which doesn't necessarily match the capitalisation of the underlying code modules that implement those modules. There's an attribute on each platform/backend module that provides the preferred capitalisation.
The actual capitalisation scheme is... inconsistent. It's mostly all lower case; except for backends and platforms that have very specific capitalisation styles (iOS, macOS, watchOS, Xcode, VisualStudio). Being lower case makes it easier to type; captialized makes it more 'humane'.
If we're going to display something to the user, I'd argue it should be consistent with the "preferred" capitalisation. Two separate issues to consider are:
- Are our current preferred capitalisations correct?
- Should modify code to be case insensitive wherever possible?
There was a problem hiding this comment.
I'm in favor of making the preferred capitalizations lower-case everywhere. This is a programming tool, and making things easy to type and to remember is more important than matching each platform's branding.
If we did this, then the pyproject.toml section name would have to become case insensitive for backward compatibility, if it isn't already.
Although it doesn't directly affect the Briefcase user, I would change the Python module names to be lower-case as well, for the same reasons as above, plus the fact that capital letters in Python module names are very unusual.
There was a problem hiding this comment.
then the pyproject.toml section name would have to become case insensitive for backward compatibility
And so would the app subdirectories.
There was a problem hiding this comment.
Ugh. Writing macos and ios is going to make my teeth itch... but you're probably right...
There was a problem hiding this comment.
I support lowercase as well...but also:
- allowing case insensitive user input for the purposes of platform/format matching
- presenting the brand's capitalization in general information to the user
- such as here when the list of supported platforms is shown;
iOSis much more recognizable thatios.
- such as here when the list of supported platforms is shown;
(Of note, this is also an issue for specifying a specific tool in briefcase upgrade [tool name] and what's displayed from briefcase upgrade --list.)
Moving to case insensitivity for filepaths and TOML names for backwards compatibility is likely to be fraught with issues, IMO.
As pointed out in #1011, the TOML names are expected to be ASCII....but that enforcement isn't bulletproof. Nonetheless, if names supported in TOML remain only ASCII, then case folding for matching is much more reliable.
As for supporting case insensitivity for filepaths....this sounds like a ticking time bomb....for instance, considering how often these filepaths are passed to other programs. I think it would be better to force users to rename the directory or delete it to continue using Briefcase. This would be similar to how the transition from ~/.briefcase to ~/.cache/briefcase was managed.
Thoughts?
I can start assessing this in the meantime.
There was a problem hiding this comment.
Based on what you're describing, it sounds like you're more in the "liberal in what you accept, conservative in what you output" camp. If we display to the user that "Android" and "iOS" are two platform options, then should either be enforcing that "Android" and "iOS" are the literal required keys, or accept "Android", "android", "ANDROID", "AnDrOiD", and anything else you might imagine are valid, either at the command line, or in a configuration file.
I think that's an equally valid approach; my only concern is the UX confusion associated with showing "Android" as a supported platform, but the default tutorial using "android". It's a little thing, but it's a potential source of confusion when we usually need to teach new users to be paranoid about exact capitalisation.
Beyond the confusion factor, the only place this policy would make a substantial difference to end users is in the directories that are generated. My tooth-itch notwithstanding, I agree with @mhsmith that being a programmer tool gives us significant license to go all-in on lower case; but whatever we do, I think we should be consistent between the "preferred" capitalisation as displayed to end users, and the "generated" capitalisation that manifests on disk.
The one thing that might make our life a little easier is that macOS is case insensitive on HFS and APFS:
(venv3.10) rkm@eunectes tmp % touch hello
(venv3.10) rkm@eunectes tmp % touch Hello
(venv3.10) rkm@eunectes tmp % touch HELLO
(venv3.10) rkm@eunectes tmp % ls
hello
Based on a hacked-together test: if I manually move a current macOS folder to macos, then re-run briefcase run, the app runs without regenerating. So - we could switch to lower case for generated filenames for iOS and macOS almost transparently.
The only fly in this ointment is windows/VisualStudio. It's the only platform that currently generates upper case in its output folders. However, I suspect the VisualStudio backend isn't extensively used (it's mostly for our own internal benefit). On top of that, we broadly encourage thinking of the generated directories as emphemeral artefacts that shouldn't be user-modified or committed to version control, so having a new folder generated in this one case isn't the end of the world, IMHO.
There was a problem hiding this comment.
The one thing that might make our life a little easier is that macOS is case insensitive on HFS and APFS
This is a configuration option, though; APFS and HFS filesystems can be configured as case-sensitive when the disk is formatted as such. Additionally, APFS supports volumes that transparently behave as directories in the host filesystem but can have different case sensitivity.
The only fly in this ointment is windows/VisualStudio
Given case insensitivity on mac isn't reliable...and for posterity...the top level build directories for iOS and macOS contain capitalization. And for iOS, the xcode directory ends up Xcode.
It wouldn't really be a microsoft product if something like case sensitivity for NTFS wasn't a true cluster....while the filesystem is technically case sensitive....many interactions with NTFS via Windows are case insensitive as the semantics are controlled by Windows APIs which use devtime-controlled configuration... Throw in a ext4 network drive or WSL and it may be difficult to say what'll happen.
That said...
I'm really only advocating case insensitivity at the command line; everywhere else, including "names" in configuration files, should only support lowercase. Case insensitive TOML really seems like a nightmare.
This approach allows backwards compatibility for any scripted commands or muscle memory...and it's already implemented and released.
As for showing branded names to users, I'm fine if that isn't done when showing the literal expected values to the user. in the help text.
Another take on presenting the platforms:
Google mobile OS Android (android)
Apple mobile OS iOS (ios)
Apple desktop OS macOS (macos)
Linux desktop OS Linux (linux)
Browser WebAssembly Web (web)
Windows desktop OS Windows (windows)
While this definitely seems ridiculously verbose right now.....when tvOS, watchOS, or wearOS is (eventually) supported, it really may not be clear to someone what those platforms actually are.
There was a problem hiding this comment.
The one thing that might make our life a little easier is that macOS is case insensitive on HFS and APFS
This is a configuration option, though; APFS and HFS filesystems can be configured as case-sensitive when the disk is formatted as such. Additionally, APFS supports volumes that transparently behave as directories in the host filesystem but can have different case sensitivity.
True - but case-insensitive is the default, and won't be changed by most users.
It wouldn't really be a microsoft product if something like case sensitivity for NTFS wasn't a true cluster....while the filesystem is technically case sensitive....many interactions with NTFS via Windows are case insensitive as the semantics are controlled by Windows APIs which use devtime-controlled configuration... Throw in a ext4 network drive or WSL and it may be difficult to say what'll happen.
That said...
I'm really only advocating case insensitivity at the command line; everywhere else, including "names" in configuration files, should only support lowercase. Case insensitive TOML really seems like a nightmare.
I'm not convinced it would be that hard - when we import the TOML, we convert any key to lowercase; and when we look up the key, we cast to lowercase. It's not nothing, but it's not hideously complex either.
However, even if did adopt case insensitive TOML, that doesn't address the filesystem problem. The issue is that are existing projects with "iOS", "macOS", "Xcode" and "VisualStudio" directories.
This approach allows backwards compatibility for any scripted commands or muscle memory...and it's already implemented and released.
As for showing branded names to users, I'm fine if that isn't done when showing the literal expected values to the user. in the help text.
I think we're getting tied in knots here.
AFAICT, we've got 3 options:
- We adopt lower case universally, and make the code adaptive to different case options (both in configuration files and with existing directories on disk)
- We adopt lower case universally, but accept the backwards incompatibility caused by existing configuration files
- We keep the existing "preferred case" handling, and use the preferred case wherever it's displayed to the user.
(3) requires the least work (read: no work outside making the help text display the preferred case); but it means the list of platforms in help text are android, iOS, macOS, windows, ....
There's possibly a continuum between (1) and (2), if a partial backwards compatibility solution is acceptable (e.g., case insensitive configuration files, but no migration path for existing directories on disk).
Honestly, at this point, I'd advocate for (3). Making a bigger push on normalising to lower case might be worthwhile, but I'd argue it's a separate issue that should be tackled as a separate ticket. We can dramatically improve the help options without also requiring a radical change to case handling.
Another take on presenting the platforms:
Google mobile OS Android (android) Apple mobile OS iOS (iOS) Apple desktop OS macOS (macOS) Linux desktop OS Linux (linux) Browser WebAssembly Web (web) Windows desktop OS Windows (windows)While this definitely seems ridiculously verbose right now.....when
tvOS,watchOS, orwearOSis (eventually) supported, it really may not be clear to someone what those platforms actually are.
I'm not sure this would be an improvement. Nobody calls it "Google mobile OS", and it's not clear to me what "Android (android)" is trying to convey.
There was a problem hiding this comment.
I'm really only advocating case insensitivity at the command line; everywhere else, including "names" in configuration files, should only support lowercase. Case insensitive TOML really seems like a nightmare.
I'm not convinced it would be that hard - when we import the TOML, we convert any key to lowercase; and when we look up the key, we cast to lowercase. It's not nothing, but it's not hideously complex either.
My concern here could arguably be considered hyperbole or overblown but it stems from corner cases:
-
Without context, Python's
lower()is locale-unaware; therefore, it can produce the wrong lowercase letter in certain cases. An recent example,'Isparta'.lower()is'isparta'instead of'ısparta'. This can be managed relatively easily for the purposes of lookup as long as the same case function is used on both sides...but user edits to TOML can break that symmetry. -
Collapsing unique keys in the TOML can create collisions. Unless detected, this could easily lead to confusion/errors.
Honestly, at this point, I'd advocate for (3). Making a bigger push on normalising to lower case might be worthwhile, but I'd argue it's a separate issue that should be tackled as a separate ticket. We can dramatically improve the help options without also requiring a radical change to case handling.
Good deal; I'm on board.
freakboy3742
left a comment
freakboy3742
left a comment
There was a problem hiding this comment.
The broad strokes of this look good to me; A few micro-bike shed suggestions for individual descriptions, and one tarpit around capitalisation.
| "Platforms:\n" | ||
| f" {', '.join([p.title() if p.islower() else p for p in platforms])}\n" |
There was a problem hiding this comment.
I think we have a big decision to make here.
The command line interface is forgiving about case, but the capitalisation matters from the perspective of configuration files - you actually need to use tool.briefcase.app.myapp.windows.VisualStudio. There's a "preferred" capitalisation for every platform and backend... which doesn't necessarily match the capitalisation of the underlying code modules that implement those modules. There's an attribute on each platform/backend module that provides the preferred capitalisation.
The actual capitalisation scheme is... inconsistent. It's mostly all lower case; except for backends and platforms that have very specific capitalisation styles (iOS, macOS, watchOS, Xcode, VisualStudio). Being lower case makes it easier to type; captialized makes it more 'humane'.
If we're going to display something to the user, I'd argue it should be consistent with the "preferred" capitalisation. Two separate issues to consider are:
- Are our current preferred capitalisations correct?
- Should modify code to be case insensitive wherever possible?
With this additional detail in the default help, I think we can probably remove the
That makes sense to me. |
- Help text will wrap at 80 characters or the current terminal width if it is less than 80 - If terminal width is not detectable, it will default to 80 (in shutil) - The final width is decreased by 2 to match argparse functionality
|
Note on help text wrapping.....bit of a rabbit hole. The text wrapping functionality from python/cpython#57015 has long documented this. python/cpython#22129 has even implemented a much more useful formatter for Sooo....I tried to find somewhat of a middle-ground in 68fce1d without overdoing it... |
|
To avoid the shenanigans with identifying the default format for a platform, I updated |
freakboy3742
left a comment
freakboy3742
left a comment
There was a problem hiding this comment.
Some gnarly argparse wrestling, but ultimately fairly straightforward - nice work!
3 participants
Changes
Help text is updated to include:
briefcaseorbriefcase -hbriefcaseorbriefcase -hbriefcase <command> -hIssues
briefcase --help#907PR Checklist: