This commit moves the remaining searcher tests from using JSON marshaled types to using JSON responses for consistency. There appears to be a weird JSON marshaling error with search.Repository that does not map `Name` field in the process.
Additionally, the test scenarios around pulling multiple pages beneath the total results have been updated to demonstrate that the REST API returns full pages in both of these cases, which is below the total number of results.
Use PrRefs.GetPRHeadLabel() instead of headBranchLabel.
Also remove headBranchLabel from CreateContext struct.
To do this, we needed a new identifier for when the head repo should be
created via a new fork of the base repo. Previously, this was done by
checking if the head repo was nil, but if we want to call
GetPRHeadLabel(), it requires a non-nil head repo to construct the
headBranchLabel. So, instead of the head repo being nil to signal
a fork, we pass a new forkHeadRepo bool in the CreateContext struct.
This also makes the decision to fork more intentional; now the decision
is made clearly instead of if the headRepo happens to be nil.
Refactor pr create by reducing flapping `isPushEnabled` between true and false.
`isPushEnabled` is now also set to `true` by default, logically aligning with the
behavior description in the help text.
This commit is focused on fixing the `searcher` tests for a few reasons:
1. Correcting the `.Total` logic in the previous commit caused changes to tests to fail
2. Tests involving results that exceed the max per page have been improved with new initialize helper, allowing testing table scenarios to be self contained
3. Tests that stub JSON response payloads have been standardized on maps rather than GitHub type primitives (Repository, Issue, Commit, Code, etc)
4. Tests had some minor formatting changes to make them easier to understand and maintain
This change restores the original logic of passing the search total count logic as is to the result.
Additionally, this undoes some of the contributor's formatting changes that increase the changed lines to review.
This bypasses the `spinner` package for the textual progress indicator for users
with the spinner disabled out of concerns for accessibility, specifically
with screen readers:
- The `spinner` package will continuously re-draw the screen. I wasn't
able to have this cause problems with my Mac screen reader, but it's
nonetheless a concern that other screen readers may not handle this
screen re-drawing well.
- The `spinner` package clears any progress indicator messages from the screen
when stopping the progress indicator or changing its label.
This is a problem because it interrupts screen readers and leaves no way
to recover what the loading message was by scrolling up in the terminal.
NOTE: this new implementation still interrupts the screen reader when
the a new label is printed, but it does not clear the screen. This makes
the loading messages recoverable, at least.
- Move env var evaluation to the factory defaults.
- Use only the label for the spinner instead of adding
"working..." every time. "working..." remains the default
when no label is provided.
Signed-off-by: Babak K. Shandiz <babakks@github.com>
This is to make sure we do not pick up the wrong file if there is a
`.txt` sequence in the middle of a job name.
