microsoft / vscode-black-formatter

Formatting support for Python using the Black formatter
https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter
MIT License
144 stars 34 forks source link

`modificationsOnly` Formats whole file when changes are at EOF #472

Closed jvacek closed 3 months ago

jvacek commented 4 months ago

If you add new code to the bottom of the file, and format modifications, it will format the entire file.

I'm not sure whether this is specific to the extension or black itself.

Version: 1.87.0
Commit: 019f4d1419fbc8219a181fab7892ebccf7ee29a2
Date: 2024-02-27T23:42:51.279Z (1 day ago)
Electron: 27.3.2
ElectronBuildId: 26836302
Chromium: 118.0.5993.159
Node.js: 18.17.1
V8: 11.8.172.18-electron.0
OS: Darwin arm64 23.2.0
Name: Black Formatter
Id: ms-python.black-formatter
Description: Formatting support for Python files using the Black formatter.
Version: 2024.0.1
Publisher: Microsoft
VS Marketplace Link: https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter
karthiknadig commented 4 months ago

Can you set this in your user settings, reload and repro the problem, and share the logs? ”black-formatter.trace.server”: “verbose”

logs should be under Output > Black Formatter.

jvacek commented 4 months ago

Here you go:

Click me ``` 2024-03-01 12:05:56.598 [info] [Trace - 12:05:56] Sending request 'textDocument/rangesFormatting - (9)'. 2024-03-01 12:05:56.598 [info] Params: { "textDocument": { "uri": "file:///Users/[redacted_path]/matching_tools/admin/missing_matchings.py" }, "ranges": [ { "start": { "line": 140, "character": 0 }, "end": { "line": 148, "character": 0 } } ], "options": { "tabSize": 4, "insertSpaces": true, "trimFinalNewlines": true, "insertFinalNewline": true } } 2024-03-01 12:05:56.603 [info] [Trace - 12:05:56] Received notification 'window/logMessage'. 2024-03-01 12:05:56.603 [info] Params: { "type": 4, "message": "/Users/jvacek/.pyenv/versions/3.10.3/envs/usermanagement/bin/python -m black -l 130 --line-ranges 141-149 --stdin-filename /Users/[redacted_path]/matching_tools/admin/missing_matchings.py -" } 2024-03-01 12:05:56.604 [info] /Users/jvacek/.pyenv/versions/3.10.3/envs/usermanagement/bin/python -m black -l 130 --line-ranges 141-149 --stdin-filename /Users/[redacted_path]/matching_tools/admin/missing_matchings.py - 2024-03-01 12:05:56.604 [info] [Trace - 12:05:56] Received notification 'window/logMessage'. 2024-03-01 12:05:56.604 [info] Params: { "type": 4, "message": "CWD formatter: /Users/[redacted_path]" } 2024-03-01 12:05:56.604 [info] CWD formatter: /Users/[redacted_path] 2024-03-01 12:05:56.660 [info] [Trace - 12:05:56] Received notification 'window/logMessage'. 2024-03-01 12:05:56.660 [info] Params: { "type": 4, "message": "reformatted /Users/[redacted_path]/matching_tools/admin/missing_matchings.py\n\nAll done! ✨ 🍰 ✨\n1 file reformatted.\n" } 2024-03-01 12:05:56.660 [info] reformatted /Users/[redacted_path]/matching_tools/admin/missing_matchings.py All done! ✨ 🍰 ✨ 1 file reformatted. 2024-03-01 12:05:56.660 [info] [Trace - 12:05:56] Received notification 'window/logMessage'. 2024-03-01 12:05:56.660 [info] Params: { "type": 4, "message": "file:///Users/[redacted_path]/matching_tools/admin/missing_matchings.py :\r\n****************************************************************************************************\r\nfrom urllib.parse import quote_plus\n\nfrom django.contrib import admin, messages\nfrom django.db.models import Count, Q\nfrom django.utils.safestring import mark_safe\n\nfrom common.mixins import NonModifiableModelAdminMixin\nfrom common.ota import OTA_BRANDDOTCOM_ID, OTA_BRANDDOTCOMSEC_ID\nfrom matching.models import OTAHotelInfo\nfrom matching_tools.models.missingmatching import MissingMatching\nfrom usermanagement.models import AccountGroup\n\nfrom .filters import (\n CountExistingValuesFieldListFilter,\n SalesforceIDExcludeFilter,\n SalesforceIDInputFilter,\n SortedCountExistingValuesFieldListFilter,\n SourceListFilter,\n)\n\n\nclass AccountGroupFilter(admin.SimpleListFilter):\n title = \"Account Group Name\"\n parameter_name = \"account_group\"\n template = \"search_box.html\"\n\n def lookups(self, request, model_admin):\n return ((),)\n\n def choices(self, changelist):\n yield next(super().choices(changelist))\n\n def queryset(self, request, queryset):\n if not self.value():\n return queryset\n\n accountgroup_ids = AccountGroup.objects.filter(name__icontains=self.value()).values(\"id\")\n return queryset.filter(\n Q(hotelinfo__accountgrouprelation__accountgroup__top_brand__in=accountgroup_ids)\n | Q(hotelinfo__accountgrouprelation__accountgroup__in=accountgroup_ids)\n )\n\n\n@admin.register(MissingMatching)\nclass MissingMatchingAdmin(NonModifiableModelAdminMixin, admin.ModelAdmin):\n show_full_result_count = False\n change_list_template = \"missing_matchings_dashboard.html\"\n list_display = (\n \"ota_display\",\n \"hotelinfo_id\",\n \"action_link\",\n \"hotelinfo_admin_url\",\n \"hotelinfo_maps_address_url\",\n \"search\",\n \"mark_non_matchable\",\n )\n list_display_links = None\n list_filter = [\n (\"priority\", CountExistingValuesFieldListFilter),\n (\"ota\", CountExistingValuesFieldListFilter),\n (\"hotelinfo__country\", SortedCountExistingValuesFieldListFilter),\n SourceListFilter,\n SalesforceIDInputFilter,\n SalesforceIDExcludeFilter,\n AccountGroupFilter,\n ]\n list_select_related = [\"matching\", \"hotelinfo__extrahotelinfo\", \"subscription\"]\n ordering = (\n \"priority\",\n \"subscription__zuora_provision_dates__rate_insight\",\n \"subscription__zuora_provision_dates__parity_insight\",\n )\n\n search_fields = [\n \"hotelinfo__name\",\n \"subscription__hotelinfo__name\",\n ]\n\n MARK_NON_MATCHABLE_NAME = \"mark-non-matchable\"\n\n def changelist_view(self, request, extra_context=None):\n if non_matchable_mm_id := request.POST.get(self.MARK_NON_MATCHABLE_NAME):\n try:\n missing_matching = MissingMatching.objects.select_related(\"hotelinfo\").get(pk=non_matchable_mm_id)\n marked_unmatchable = missing_matching.action_mark_as_non_matchable(request.user)\n if marked_unmatchable:\n hotelinfo_admin_link = f'{missing_matching.hotelinfo}'\n messages.success(\n request, mark_safe(f\"{missing_matching.ota_id} was marked non-matchable for {hotelinfo_admin_link}\")\n )\n except MissingMatching.DoesNotExist:\n messages.info(request, \"Missing matching does not exist anymore.\")\n response = super().changelist_view(request, extra_context)\n cl = ctx[\"cl\"] if (ctx := getattr(response, \"context_data\", None)) else self.get_changelist_instance(request)\n queryset = cl.queryset.values(\"ota\").annotate(count=Count(\"ota\")).values_list(\"ota\", \"count\").order_by(\"-count\")\n ota_mapper = dict(queryset)\n response.context_data.update({\"ota_overview\": ota_mapper})\n return response\n\n def get_queryset(self, request):\n return MissingMatching.admin_objects.all()\n\n def hotelinfo_admin_url(self, obj):\n url = f\"{obj.hotelinfo.admin_url}#matching\"\n hotel_name = f\"{obj.hotelinfo}\" if obj.hotelinfo_id == obj.subscription.hotelinfo_id else obj.hotelinfo\n return mark_safe(f'{hotel_name}')\n\n def hotelinfo_maps_address_url(self, obj):\n address = f\"{obj.hotelinfo.latitude},{obj.hotelinfo.longitude}\"\n if obj.hotelinfo.extrahotelinfo.address:\n address = obj.hotelinfo.extrahotelinfo.address.replace(\",\", \" \")\n url = f\"https://www.google.com/maps/search/?api=1&query={quote_plus(address)}\"\n return mark_safe(f'{address}')\n\n @admin.display(description=\"Mark unresolvable\")\n def mark_non_matchable(self, obj) -> str:\n if obj.source == MissingMatching.Source.PRIMARY_BRAND:\n return \"\"\n return mark_safe(f'
karthiknadig commented 4 months ago

@jvacek This might be an issue on black itself. We are running it with the ranges for the modified part, as you can see here in the logs:

/Users/jvacek/.pyenv/versions/3.10.3/envs/usermanagement/bin/python -m black -l 130 --line-ranges 141-149 --stdin-filename /Users/[redacted_path]/matching_tools/admin/missing_matchings.py -

but black is formatting the entire file.

The way to repro this locally, and exactly like the extension does with unsaved file is like this:

  1. make changes to file that should be formatted by black outside of VS Code
  2. Note the line you want black to format. be sure that there are other areas outside of the range that can be formatted as well.
    cat <unformatted file path> | black -l 130 --line-ranges <start-line#>-<end-line#> --stdin-filename <unformatted file path> - 

    This will pass the file contents to black using stdin and should behave exactly like how the extension triggers black. If the content that is printed out is fully formatted then it is a bug with black.

jvacek commented 4 months ago

@karthiknadig Thanks, I have successfully repro'd this with black itself (which i should have done to begin with hah 🙃). I opened an issue on the Black repo.

Depending on the outcome of the ticket above, would you consider adapting the input to black to not pass the last line of the file?

karthiknadig commented 4 months ago

Depending on the outcome of the ticket above, would you consider adapting the input to black to not pass the last line of the file?

I don't fully understand, could you elaborate? are you suggesting to make the end-line value exclusive? or are you suggesting modify the content sent to black.

jvacek commented 3 months ago

@karthiknadig the issue was fixed in black, and is now available in 24.3.0. How often does the bundled version get bumped?

karthiknadig commented 3 months ago

We will publish an update to this with the new black soon (likely early next week)

karthiknadig commented 3 months ago

This is in pre-release