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

Add skip first run prompt cli flag #104

Merged

Conversation

timon-schelling
Copy link
Contributor

Adds a --skip-first-run-prompt flag.
"Disables the welcome message shown on the first launch of tere, which prompts the user to update their shell configuration for proper directory changing functionality. Use this flag if you have already configured your shell or want to bypass the prompt for other reasons."

See timon-schelling@eba4049#comments for the discussion that lead up to this.

@mgunyho The first commit contains all feature relevant code.
In the second commit I've updated the textwarp dependency, otherwise I'm not able to build master or this pr. error on master:

error[E0635]: unknown feature `stdsimd`
  --> /home/codespace/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ahash-0.7.6/src/lib.rs:33:42
   |
33 | #![cfg_attr(feature = "stdsimd", feature(stdsimd))]
   |                                          ^^^^^^^

For more information about this error, try `rustc --explain E0635`.
error: could not compile `ahash` (lib) due to 1 previous error

After updating textwarp I get a failing test both on main plus the second commit and for this pr.
RUST_BACKTRACE=1 cargo test --no-fail-fast --locked:

failures:

---- app_state::tests::test_case_sensitive_mode_change stdout ----
thread 'app_state::tests::test_case_sensitive_mode_change' panicked at src/app_state.rs:1711:9:
assertion `left == right` failed
  left: [1]
 right: [2]
stack backtrace:
   0: rust_begin_unwind
             at /rustc/c2f74c3f928aeb503f15b4e9ef5778e77f3058b8/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/c2f74c3f928aeb503f15b4e9ef5778e77f3058b8/library/core/src/panicking.rs:74:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/c2f74c3f928aeb503f15b4e9ef5778e77f3058b8/library/core/src/panicking.rs:367:5
   4: tere::app_state::tests::test_case_sensitive_mode_change
             at ./src/app_state.rs:1711:9
   5: tere::app_state::tests::test_case_sensitive_mode_change::{{closure}}
             at ./src/app_state.rs:1696:41
   6: core::ops::function::FnOnce::call_once
             at /rustc/c2f74c3f928aeb503f15b4e9ef5778e77f3058b8/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/c2f74c3f928aeb503f15b4e9ef5778e77f3058b8/library/core/src/ops/function.rs:250:5

I guess this could be a rustc version mismatch but I'm not sure. What rustc version are you building against? Is this test working on master for you?

timon-schelling referenced this pull request in timon-schelling/tere Sep 10, 2024
@timon-schelling
Copy link
Contributor Author

could this be related? #90

@mgunyho
Copy link
Owner

mgunyho commented Sep 11, 2024

Thanks! I'll look at the PR later today.

That's strange with the build errors. I tried now on master with rustc 1.77.1 (7cf61ebde 2024-03-27) (on Fedora 40) and it worked fine. I'll try to update rustc and see if it fails after that.

The failure of test_case_sensitive_mode_change indeed seems to be the same as #90, and it's still a mystery to me.

@mgunyho
Copy link
Owner

mgunyho commented Sep 11, 2024

The tests work fine for me also on rustc 1.81.0 (eeb90cda1 2024-09-04) (although I get a couple of new warnings).

@timon-schelling
Copy link
Contributor Author

I tried it on GitHub code space and had the same problems like I described

@timon-schelling
Copy link
Contributor Author

timon-schelling commented Sep 11, 2024

Do you have suggestions on how to move forward? #90 seems to be environment dependent, as a test that should not be. On NixOS I'm able to build and run all tests. See https://github.com/timon-schelling/timonos/blob/7ed1566c035a98bcec317974698ff6f0060e70ae/overlays/tere/overlay.nix
I'm really confused.

@mgunyho
Copy link
Owner

mgunyho commented Sep 13, 2024

Hi, I checked your code and the PR looks good! Thanks for also including the tests. Could you please

  1. rebase on to develop (that branch already has the update of textwrap to 0.16, although I couldn't reproduce the compilation issue you had) and
  2. write a short note in CHANGELOG.md?

I'll then make a new release over the weekend.

The mysterious failling test is the same as #90 so we can leave that for the future.

@mgunyho mgunyho changed the base branch from master to develop September 13, 2024 07:31
@timon-schelling timon-schelling force-pushed the add-skip-first-run-prompt-cli-flag branch from 576dfab to b6f32d7 Compare September 13, 2024 08:21
@timon-schelling timon-schelling marked this pull request as ready for review September 13, 2024 08:23
mgunyho added a commit that referenced this pull request Sep 15, 2024
@mgunyho mgunyho merged commit 999aafc into mgunyho:develop Sep 15, 2024
mgunyho added a commit that referenced this pull request Sep 15, 2024
- Add a cli flag named `--skip-first-run-prompt` that disables the welcome message shown on the first launch of tere, which prompts the user to update their shell configuration for proper directory changing functionality. (Thanks to Timon Schelling, Github #104)
- Update dependencies
mgunyho added a commit that referenced this pull request Sep 15, 2024
- Add a cli flag named `--skip-first-run-prompt` that disables the welcome message shown on the first launch of tere, which prompts the user to update their shell configuration for proper directory changing functionality. (Thanks to Timon Schelling, Github #104)
- Update dependencies

(Originally released as 1.5.2, but this was inconsistent with semver)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants