Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Omit _All Files_ filter in Electron on Linux #11325

Merged
merged 2 commits into from
Jun 28, 2022

Conversation

planger
Copy link
Contributor

@planger planger commented Jun 22, 2022

What it does

Omits adding the All Files filter, if we are on Linux and no other filter is specified. Otherwise, we run into #11321 which causes an extension filter ['*'] to hide files without extensions. Once we are on Electron >18, we should be able to add the All Files filter again in all cases, as this Electron bug seems to be resolved in Electron above >18.

Fixes #11321

Change-Id: Iec3120a6260f20b7f0d42c091aa00bde33fa915a
Signed-off-by: Philip Langer [email protected]

How to test

  1. Use Ubuntu Linux
  2. Start Electron Example App
  3. Open Settings (UI)
  4. Navigate to Terminal › External: Windows Exec
  5. Click Browse
  6. Navigate to a folder that contains a file without an extension
  7. Observe how the file without extension shows up in the dialog

We should also ensure that other OSs aren't affected (OSX and Windows), as well as that the All Files filter still shows up when a specific filter is specified. Unfortunately, I could only test the latter myself (when specific filters are specified) but not the former, because I don't have a Windows or OSX machine.

Review checklist

Reminder for reviewers

We shouldn't add an _All Files_ filter, if we are on Linux and
no other filter is specified. Otherwise, we run into
eclipse-theia#11321
which causes an extension filter `['*']` to hide files without
extensions. Once we are on Electron >18, we should be able
to add the _All Files_ filter again in all cases, as this Electron
bug seems to be resolved in Electron above >18.

Fixes eclipse-theia#11321

Change-Id: Iec3120a6260f20b7f0d42c091aa00bde33fa915a
Signed-off-by: Philip Langer <[email protected]>
@colin-grant-work colin-grant-work self-requested a review June 22, 2022 20:25
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix appears sound. 👍 Optionally, the comment could be more concise.

Co-authored-by: colin-grant-work <[email protected]>
@JonasHelming JonasHelming added this to the 1.27.0 milestone Jun 28, 2022
@JonasHelming JonasHelming merged commit 8d46c9b into eclipse-theia:master Jun 28, 2022
@vince-fugnitto vince-fugnitto added electron issues related to the electron target OS/Linux issues related to the Linux OS labels Jun 30, 2022
@planger planger deleted the planger/issues/11321 branch February 3, 2023 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron issues related to the electron target OS/Linux issues related to the Linux OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Files without extension are filtered in ElectronFileDialogService even though no filter is provided
4 participants