vscode icon indicating copy to clipboard operation
vscode copied to clipboard

Fix recent history exclude not working

Open Legend-Master opened this issue 1 year ago • 1 comments

Fix #157395

Hey sorry. I was on vacation. Something I'm curious about... if you add this to search.exclude do they go away? I doubt it but I just want to make sure.

As @TylerLeonhardt mentioned, it turns out to be a bug, exclude settings will only affect it on configuration changes, not even on history/window reload

Legend-Master avatar Sep 20 '22 05:09 Legend-Master

By the way, both files.exclude and search.exclude should work, but it's slightly different than how it works on file explorer and search panel, you have to use a file match pattern instead of directory one e.g. **/.git will not work but **/.git/** will

Legend-Master avatar Sep 20 '22 06:09 Legend-Master

Actually now I remembered why the code works as is: there is still users out there (including me and some from our team) that navigate editors exclusively via the file history. This is cool because you only need a single keybinding and UI piece for all matters of editor accessing and opening.

If we were to change how history works, this scenario would break:

  • open an editor from an excluded resource
  • navigate back in history to the previous editor
  • try to navigate back to the excluded resource
  • 🐛 you can no longer do that even though the editor is open

I will push a change to go back to the original solution but will revisit this when I am back from leave.

bpasero avatar Sep 27 '22 11:09 bpasero

Yep, that's what I said 🤣

And if we actually exclude these files from the get go, we can't use quick open to switch to these files while they are still opened which is something you can do to non files (e.g. settings page), so I'm not sure would we want that

Legend-Master avatar Sep 27 '22 11:09 Legend-Master

I am sorry but have decided to not accept the change in this area and since we are in grooming iteration, I am closing this PR.

bpasero avatar Dec 06 '22 06:12 bpasero

I am sorry but have decided to not accept the change in this area and since we are in grooming iteration, I am closing this PR.

Well... That's pretty sad, could you tell me why if you will

And does that mean this 'bug' will not be fixed?

Should I reopen this after 'grooming iteration' or just leave it?

Legend-Master avatar Dec 06 '22 08:12 Legend-Master

I was not convinced to force exclude history entries that are opened, such as https://github.com/microsoft/vscode/pull/161300#issuecomment-1259366596 indicated.

bpasero avatar Dec 06 '22 16:12 bpasero

Didn't you already rollback to the original solution which will only remove the excluded file's history after closing the editor(tab) ?

(this solution won't break any feature as long as your editor(tab) is still open)

Legend-Master avatar Dec 07 '22 02:12 Legend-Master

Even then, I am not sure this might break peoples assumption. For example users might have setups where editors are closed:

  • there is a setting to auto-close editors based on a limit
  • I for example always close all editors before starting a new task

In essence, with this change the editor history is less deterministic and may no longer reflect the actual history of editors that were opened.

bpasero avatar Dec 07 '22 07:12 bpasero

Well, ok then, sounds reasonable...

Legend-Master avatar Dec 07 '22 07:12 Legend-Master