[Diff] Add tab context menu entry to diff 2 tabs

[keith-hall]

Fixes #3313

[jrappen]

Thanks for this.

• The mixed use of different quotes for strings was unexpected. Could you choose one?
• Might make sense to use f-strings in line 58.
• If you got extra time, maybe you could add typing info since we're already at Py3.8 for this package.

[deathaxe]

None of ST's core/default plugins uses typing so far. I don't see any reason for them here.

[jwortmann]

Traceback (most recent call last):
  File "C:\Program Files\Sublime Text\Lib\python38\sublime_plugin.py", line 1480, in run_
    return self.run(**args)
  File "C:\Users\jwortmann\AppData\Roaming\Sublime Text\Packages\Diff\diff.py", line 58, in run
    show_diff_output(diff, None, self, os.path.basename(files[1]) + " -> " + os.path.basename(files[0]))
  File "C:\Users\jwortmann\AppData\Roaming\Sublime Text\Packages\Diff\diff.py", line 109, in show_diff_output
    v = win.create_output_panel('unsaved_changes')
AttributeError: 'DiffFilesCommand' object has no attribute 'create_output_panel'

This happens for me on this branch if you use the regular Diff files... entry from the sidebar context menu. The show_diff_output function seems to expect a sublime.Window as the third argument, but recieves a sublime_plugin.WindowCommand in line 58. If you change self to self.window there, it still doesn't work and fails on the second argument, which is supposed to be a sublime.View, but acutally is None.

---

If I set "diff_changes_to_buffer": true in the settings, the tab title appears odd for diffs of two existing files from the tab context menu: there is an unexpected "Unsaved Changes:" prefix and a part of the filename is removed if both files start with a similar filename. For example:

---

In general I like the idea from this PR. The functionality already exists, and I always wondered why diffs are only available for files from the side bar, instead of all views. No more need to install 3rd-party packages for basic diffs :)

[keith-hall]

Thanks for the feedback all, it seems I didn't do enough testing after making some quick changes 😅

---

@jrappen

• The mixed use of different quotes for strings was unexpected. Could you choose one?

I like to think that double quotes are being used for human-readable/localizable strings, and single quotes for internal stuff, so I made some changes in that direction. But if you want a single/uniform quote type, let's decide which we prefer and switch them all.

---

@jwortmann should be fixed now, thanks for the detailed report

[jrappen]

I'm just thinking loud, Keith. It was unexpected for me.

I use single quotes. Your reasoning seems fine. Whatever works and/or people are used to.

[deathaxe]

This change definitly adds value to the existing plugin. I've used it every here and then without issues. Not sure if a new context menu item is something @sublimehq should decide about, but I am fine with it as is.

[jwortmann]

I can confirm that it works well and doesn't raise an error anymore, but I still have two suggestions for small possible improvements:

1. With the "diff_changes_to_buffer" setting activated, there is a leading backslash (on Windows) in the filepaths of the tab title, which is especially noticeable if the files are in the same folder, e.g. \file1.py -> \file2.py. I'd prefer how the Diff Files... feature from the side bar shows it without the backslash like file1.py -> file2.py. So I would suggest

   -          common_path_length = len(os.path.commonpath(view_names))
   +          common_path_length = len(os.path.commonpath(view_names)) + 1

in line 168.

2. Maybe use something like "View (15)" or "Unsaved view (15)" as the fallback names for the tab title and diff header, instead of "from_file" and "to_file"?

- def get_name_for_view(view, fallback_name):
-     return view.file_name() or view.name() or fallback_name
+ def get_name_for_view(view):
+     return view.file_name() or view.name() or "Unsaved view ({})".format(view.id())

[keith-hall]

@jwortmann done :) I tweaked the common path logic slightly to also handle when the only part in common is / i.e. between /home/user/file and /etc/file - to keep it, and to ensure that on Windows D:\file and C:\file would not cut out the drive letter as it would if I used your suggestion directly.

[jwortmann]

Perfect 👌

[Ultra-Instinct-05]

Been using it for sometime & seems solid so far.

[michaelblyons]

Merge?

[deathaxe]

Waiting for some comment from sublimehq about the new plugin functionality.

I am fine with it, though I'd be happy to also be able to diff views from different groups.

[BenjaminSchaaf]

Thanks for the work on this, it's a good addition. The command should also be added to Tab Sidebar Context.sublime-menu so it can work in the side-bar.