black icon indicating copy to clipboard operation
black copied to clipboard

added python version to parse error message

Open nnrepos opened this issue 1 year ago • 5 comments

Description

improve parsing error message to make sure users know which version it was run on (fixes #3294).

Checklist - did you ...

  • [x] Add an entry in CHANGES.md if necessary?
  • [x] Add / update tests if necessary?
  • [x] Add new / update outdated documentation?

nnrepos avatar Oct 10 '22 13:10 nnrepos

diff-shades reports zero changes comparing this PR (fc39bca8199c8218da8d19b6744e3e0234816733) to main (d338de7f687e90e292d949d2acd301f59fe5cdf8).


What is this? | Workflow run | diff-shades documentation

github-actions[bot] avatar Oct 10 '22 14:10 github-actions[bot]

I feel like this will create more user confusion:

% black --target-version py36  -c 'a++'
a++
error: cannot format <string>: Cannot parse (python 3.0): 1:3: a++
% black --target-version py39  -c 'a++'
a++
error: cannot format <string>: Cannot parse (python 3.7): 1:3: a++

Users are going to wonder why it says "3.0" or "3.7" when they never asked for that version.

ok, can you help me understand why we parse python 3.0, when a user asks to parse py36?

nnrepos avatar Oct 26 '22 05:10 nnrepos

ok, can you help me understand why we parse python 3.0, when a user asks to parse py36?

We don't have separate grammars for every version. For py36 we use a grammar with version set to (3, 0).

JelleZijlstra avatar Oct 26 '22 14:10 JelleZijlstra

ok, can you help me understand why we parse python 3.0, when a user asks to parse py36?

We don't have separate grammars for every version. For py36 we use a grammar with version set to (3, 0).

ok, so do we have an access to the version that the user provided somewhere?

nnrepos avatar Oct 26 '22 21:10 nnrepos

It's the target version, which gets passed as a command-line parameter and thence as an argument to main().

JelleZijlstra avatar Oct 27 '22 00:10 JelleZijlstra

hey @JelleZijlstra @felix-hilden, i've made some changes, please let me know if this is helpful 😄

nnrepos avatar Nov 04 '22 17:11 nnrepos

I'm still not enthusiastic about this:

  • Hardcoding the list of versions is a little hacky and hard to maintain
  • I'm afraid the error message may still be confusing for users, but don't have a better suggestion

JelleZijlstra avatar Nov 07 '22 05:11 JelleZijlstra

I'm still not enthusiastic about this:

  • Hardcoding the list of versions is a little hacky and hard to maintain
  • I'm afraid the error message may still be confusing for users, but don't have a better suggestion
  • i've used the same hard-coding that is used by the parser, since there weren't any constants/functions which can be used to do that. that can be refactored of course.
  • i think users are smart enough to understand what a range of numbers is, we're all programmers here...

feel free to close this if you're not interested

nnrepos avatar Nov 07 '22 06:11 nnrepos