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 for bug showing preview icon for unknown files #1768

Merged
merged 3 commits into from
Jun 23, 2024

Conversation

armartinez
Copy link
Contributor

Updated file type computed property so it defaults to text if file type is unknown.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

@bombardier200
Copy link
Contributor

Fix swiftlint errors then this should be good to merge!

@Wouter01 Wouter01 self-requested a review June 23, 2024 20:53
@bombardier200 bombardier200 merged commit e8b1bf5 into CodeEditApp:main Jun 23, 2024
2 checks passed
@armartinez armartinez deleted the fix-preview-unknown-files branch June 24, 2024 07:45
@plbstl
Copy link
Contributor

plbstl commented Jul 1, 2024

This PR may create an unpredictable behavior.

If the file type is unknown, CodeEdit should utilize QuickLook.

Also:

  • NonTextFileView uses the image and pdf conformances to correctly display the opened file with QuickLook. Other conformances (e.g. video) can also be added later on for finer-grained customization.
  • The CodeFileDocument.utType documentation explains that python scripts (as well as other text files) will return a text UTType.

Knowing that, a good compromise can be to check for UTType conformances in the NonTextFileView instead. This is what I did initially but changed it to make the code a bit more readable and understandable at first glance.

One way of implementing this is to update the if let fileURL block in NonTextFileView to this:

if let fileURL = fileDocument.fileURL {
    if let utType = fileDocument.utType {
        if utType.conforms(to: .image) {
            ImageFileView(fileURL)
                .modifier(UpdateStatusBarInfo(withURL: fileURL))
        } else if utType.conforms(to: .pdf) {
            PDFFileView(fileURL)
                .modifier(UpdateStatusBarInfo(withURL: fileURL))
        }
    } else {
        AnyFileView(fileURL)
            .modifier(UpdateStatusBarInfo(withURL: fileURL))
    }
} else {
    ZStack {
        Text("Cannot retrieve URL to the file you opened.")
    }
}

And then update the documentation for CodeFileDocument.utType:

/// The type of data this file document contains.
///
/// If its text content is not nil, a `text` UTType is returned.
///
/// - Note: The UTType doesn't necessarily mean the file extension, it can be the MIME
/// type or any other form of data representation.
var utType: UTType?

With these changes, the tests still pass, and the behavior remains predictable.

For issues regarding icons showing up for files like .gitignore, license, etc, #1724 provided a fix.

@bombardier200
Copy link
Contributor

Some notes that I found when doing investigation was that not all text file actually return the .text as the uttype, such as JSX returns a very weird file type. Also, because CodeEdit is a text editor first, I believe it should always default to text if unknown. The unpredictable behavior you are talking about, can you maybe share why/ what file this might happen on? I did a pretty exhaustive list before this merge and had great success with it picking textview or Quick Look.

@austincondiff
Copy link
Collaborator

If the file type is unknown, CodeEdit should utilize QuickLook.

Not necessarily. Instead we should check if the file can be rendered as plain text regardless of the file type. Otherwise we use QuickLook.

CodeEdit is first and foremost a text editor so if at all possible we should try to render as text first so the user can edit similar to VS Code and others.

@plbstl
Copy link
Contributor

plbstl commented Jul 1, 2024

The main change is in https://github.com/CodeEditApp/CodeEdit/pull/1768/files#diff-fae281163b35bcdac4e0ef2c73743af584f33da0a45280ca99396a3e988a069c.

The PR did not actually change the default file type to text when its UTType is unknown. What it did was remove the conformance checks needed to correctly display files with QuickLook.

The change that was suggested only adds the removed checks to the NonTextFileView. Without these checks, smaller images will be scaled up to fit editor view when previewing in CodeEdit.

CodeEdit is a text editor first

I agree. I think I should've reworded the first comment. The main change is for NonTextFileView to work correctly.

I did a pretty exhaustive list

Yes. So far, a very exhaustive list of files load correctly as text and non-text files.

@bombardier200
Copy link
Contributor

Okay I think we are the same page, I think your purposed change makes sense If you want to put in a PR for it.

@plbstl
Copy link
Contributor

plbstl commented Jul 1, 2024

Okay, will do that right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants