SublimeText / PackageDev

Tools to ease the creation of snippets, syntax definitions, etc. for Sublime Text.
MIT License
436 stars 83 forks source link

Settings: Fix duplicated value completions #259

Closed deathaxe closed 5 years ago

deathaxe commented 5 years ago

Btw. nice changes done in the recent commits. To continue completing each others ideas, this PR proposes some fixes for regressions introduced by 30983c799d9b048bb1ab5795fe117c70c1969f8e which cause:

  1. duplicated completions (e.g.: "caret_style") and
  2. missing (default) marks for list values (e.g.: "indent_guide_options").

The method _completions_from_comment() needs to do default marking in order to prevent _completions_from_default() to add the same value.

The format_completion_item() function needs to handle list type default values when setting is_default.

Force _completions_from_default() to mark values added from the default list by passing is_default=True to the format_completion_item(). It's just an optimization to avoid the value in default check.

FichteFoll commented 5 years ago

Good catches.

The method _completions_from_comment() needs to do default marking in order to prevent _completions_from_default() to add the same value.

For some reason I thought it already did that, but seeing how it never had the default available, this is kind of silly. I'll blame it being late yesterday.

The format_completion_item() function needs to handle list type default values when setting is_default.

Totally forgot about that.

Yesterday I thought that tests for this part of PackageDev would be neat because it's easy to regress like this, but the thought of the required effort of making the code testable put a quick end to that.

Also, while we're on this topic: I also thought about checking whether we're inside a settings file that is based on Preferences.sublime-settings to check whether scheme completions should be provided, but it's not really worth the effort and this setting name is unlikly to be used for something else. Similarly for theme completions, although those are only valid inside an actual Preferences.sublime-settings file.