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

Fix warning when used in elixir 1.16 #55

Closed
wants to merge 2 commits into from

Conversation

JonathanTron
Copy link

Elixir 1.16.0 String.slice/2 started warning about the range having negative steps:

iex(1)> String.slice("demo", 1..-1)
warning: negative steps are not supported in String.slice/2, pass 1..-1//1 instead
  (elixir 1.16.0) lib/string.ex:2368: String.slice/2
  (elixir 1.16.0) src/elixir.erl:405: :elixir.eval_external_handler/3
  (stdlib 5.2) erl_eval.erl:750: :erl_eval.do_apply/7
  (elixir 1.16.0) src/elixir.erl:378: :elixir.eval_forms/4
  (elixir 1.16.0) lib/module/parallel_checker.ex:112: Module.ParallelChecker.verify/1
  (iex 1.16.0) lib/iex/evaluator.ex:331: IEx.Evaluator.eval_and_inspect/3
  (iex 1.16.0) lib/iex/evaluator.ex:305: IEx.Evaluator.eval_and_inspect_parsed/3
  (iex 1.16.0) lib/iex/evaluator.ex:294: IEx.Evaluator.parse_eval_inspect/4
  (iex 1.16.0) lib/iex/evaluator.ex:187: IEx.Evaluator.loop/1
  (iex 1.16.0) lib/iex/evaluator.ex:32: IEx.Evaluator.init/5
  (stdlib 5.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3

This fixes it.

Elixir 1.16.0 `String.slice/2` started warning about the range having negative steps:

```elixir
iex(1)> String.slice("demo", 1..-1)
warning: negative steps are not supported in String.slice/2, pass 1..-1//1 instead
  (elixir 1.16.0) lib/string.ex:2368: String.slice/2
  (elixir 1.16.0) src/elixir.erl:405: :elixir.eval_external_handler/3
  (stdlib 5.2) erl_eval.erl:750: :erl_eval.do_apply/7
  (elixir 1.16.0) src/elixir.erl:378: :elixir.eval_forms/4
  (elixir 1.16.0) lib/module/parallel_checker.ex:112: Module.ParallelChecker.verify/1
  (iex 1.16.0) lib/iex/evaluator.ex:331: IEx.Evaluator.eval_and_inspect/3
  (iex 1.16.0) lib/iex/evaluator.ex:305: IEx.Evaluator.eval_and_inspect_parsed/3
  (iex 1.16.0) lib/iex/evaluator.ex:294: IEx.Evaluator.parse_eval_inspect/4
  (iex 1.16.0) lib/iex/evaluator.ex:187: IEx.Evaluator.loop/1
  (iex 1.16.0) lib/iex/evaluator.ex:32: IEx.Evaluator.init/5
  (stdlib 5.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3
```

This fixes it.
@nathany-copia
Copy link

Thanks @JonathanTron. I'm going to use your branch for now.

@TheFirstAvenger
Copy link

@JonathanTron thank you for this fix! Looking forward to it being merged into main.

@ddengler
Copy link

Does this break with the set elixir: "~> 1.2", compatibility? Is this even correct or should this also be changed? Could find //step syntax only in elixir 1.12 and up on a quick glance - which is also the oldest version with active security patches

@JonathanTron
Copy link
Author

JonathanTron commented Jan 29, 2024

Hi @ddengler, from a try to run on elixir 1.2:

# mix deps.get
** (Mix.Config.LoadError) could not load config config/config.exs
    ** (CompileError) config/config.exs:3: module Config is not loaded and could not be found
    (elixir) lib/code.ex:168: Code.eval_string/3
    (mix) lib/mix/config.ex:150: Mix.Config.read!/1
    (mix) lib/mix/tasks/loadconfig.ex:36: Mix.Tasks.Loadconfig.load/1
    (mix) lib/mix/tasks/loadconfig.ex:27: Mix.Tasks.Loadconfig.run/1

So I'm not sure the compatibility with 1.2 is so important :), I think changing it to "~> 1.12" is for the best.

@nathany-copia
Copy link

How many versions back do you want/need to support?

Unfortunately I don't see in the release notes or the documentation for slice and Range when the newer syntax was first supported: https://github.com/elixir-lang/elixir/releases/tag/v1.16.0

As for Config:

the Config module in Elixir was introduced in v1.9 as a replacement to use Mix.Config, which was specific to Mix and has been deprecated.

So 1.9 may be worth testing as a minimum?

Personally I'd only support the last 2 or 3 minor releases (e.g. 1.16.x, 1.15.x, 1.14.x).

@ddengler
Copy link

@JonathanTron @nathany-copia I agree. We only use 1.15 and 1.16. – as this seems to be a basic (in many toolboxes) dependency, I think this should be addressed by changing the minimum version to the earliest working release.

@paulo-silva
Copy link

Hey guys, is something holding this PR to be merged?

@rhblind
Copy link

rhblind commented Feb 7, 2024

Would also love to see this merged :)

@ViseLuca
Copy link

@rubemz can you helps us merging this one?

@nathany-copia
Copy link

It looks like #62 makes the same change and was merged.

@JonathanTron
Copy link
Author

Thanks @nathany-copia , I didn't see it yet. I'm closing this one then.

@JonathanTron JonathanTron deleted the patch-1 branch February 21, 2024 07:30
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.

None yet

7 participants