files icon indicating copy to clipboard operation
files copied to clipboard

Add preview column to Column View

Open shiruken1 opened this issue 5 months ago • 73 comments

Fixes #1601 and gives text preview for "text/*" and "application/sql" mimetypes

shiruken1 avatar Jul 13 '25 11:07 shiruken1

Screenshot from 2025-07-29 13-44-49

@jeremypw @alainm23 wutcha'll think? I'd like to pad the Box to the middle of the colpane, but this is my first time coding in Vala (Love it!) and using the Gtk/Gdk libs, which i haven't figured out how to "pad" like in CSS. Open to feedback!

P.S: Oh yeah, and padding between the info_grid items, as well

shiruken1 avatar Jul 29 '25 18:07 shiruken1

@leonardo-lemos I took it out of draft mode a little too quickly and forgot about text preview. No clue what pattern to follow to pull that off, and I know it's technically out of scope for the issue, but I feel it's in the spirit of mirroring OSX' preview panel, yes?

Thoughts?

shiruken1 avatar Jul 29 '25 19:07 shiruken1

@shiruken1 I applaud you for having the courage to get to grips with Files code - its not easy! The screenshot looks very promising. I'll take some time to go through the code and get back to you with specific issues. I suspect we will want to have a preference switch to turn this feature on and off. @danirabbit Are you willing to extend Files in this way? I know you have expressed doubts about file previewing before but there seems to be a significant demand for it. I have no objection if implemented well.

jeremypw avatar Jul 30 '25 12:07 jeremypw

@shiruken1 I applaud you for having the courage to get to grips with Files code - its not easy! The screenshot looks very promising. I'll take some time to go through the code and get back to you with specific issues. I suspect we will want to have a preference switch to turn this feature on and off. @danirabbit Are you willing to extend Files in this way? I know you have expressed doubts about file previewing before but there seems to be a significant demand for it. I have no objection if implemented well.

Thanks!

Thankfully my C# coding came in handy to get my eyes adjusted (10+ years of Node and Python) but I gotta say the codebase is neat and tidy for the most part, making it easy for me to pattern-match.

I did wanna propose pulling construct_info_panel out of PropertiesWindow and placing it in AbstractPropertiesDialog, so DetailsColumn could piggy-back off of as much as possible to keep the code DRY. Lmk!

shiruken1 avatar Jul 30 '25 12:07 shiruken1

I suggest you install io.elementary.vala-lint from https://github.com/vala-lang/vala-lint.git so you can de-lint the code locally. There are a few warnings atm:

../src/View/ColumnView.vala
   96.14    error   Expected space before paren                     space-before-paren
   96.30    error   Expected space before paren                     space-before-paren
../src/View/DetailsColumn.vala
   28.33    error   Expected space before paren                     space-before-paren

3 errors, 0 warnings

jeremypw avatar Jul 30 '25 13:07 jeremypw

All set. I went ahead and fixed the line-length warnings in the files I worked with. Hope that's ok.

shiruken1 avatar Jul 30 '25 17:07 shiruken1

@shiruken I had quite a lot of comments on the code, mostly stylistic. I pushed a PR to your branch as it was easier than having a lot of comments. Please ask if there is something you do not understand or find I broke something :smile:

jeremypw avatar Jul 30 '25 18:07 jeremypw

@jeremypw thank you so much for that! I thought I had followed the Code Style guide closely, but being the first time I guess a second pair of seasoned eyes would catch way more lol

@danirabbit here's the latest screenshot Screenshot from 2025-07-30 14-09-27

shiruken1 avatar Jul 30 '25 20:07 shiruken1

@shiruken1 You did remarkably well for your first attempt with Vala/elementaryos!

jeremypw avatar Jul 30 '25 21:07 jeremypw

@jeremypw Thanks again! Could I push my luck just a little bit? I'd really like text preview as I have to deal with text files a lot (logs, SQL, code, etc)

A) Text preview ok? B) Could I trouble you for a hand with the text_window's incessant shoving down the info_grid? I can't figure it out :(

Screenshot from 2025-07-31 07-43-31

shiruken1 avatar Jul 31 '25 12:07 shiruken1

P.S: no, SQL is not code... it's hell :sob:

shiruken1 avatar Jul 31 '25 12:07 shiruken1

@jeremypw All set! :tada: Screenshot from 2025-08-02 10-18-11

shiruken1 avatar Aug 02 '25 15:08 shiruken1

Screenshot from 2025-08-02 13-47-21 Screenshot from 2025-08-02 13-46-35

I'm thinking halign = CENTER looks better :hugs:

shiruken1 avatar Aug 02 '25 18:08 shiruken1

Buttons are usually aligned at start or end when on the last line of a dialog but @danirabbit has final say on design.

jeremypw avatar Aug 03 '25 09:08 jeremypw

@jeremypw Hello again! Question: Cassidy stated in the issue " It especially will feel nice in the Column view, but should work well everywhere." Should I start working on something for the other views? I have nothing to go on, so I'd just be sticking to a Box that holds the DetailsColumn. Wutcha think?

shiruken1 avatar Aug 11 '25 12:08 shiruken1

Should I start working on something for the other views

I should wait to see what reception this PR gets. Hopefully much of what you have done here can be re-used in other views.

Don't forget to implement an option (default = off) to show previews.

jeremypw avatar Aug 11 '25 12:08 jeremypw

Should I start working on something for the other views

I should wait to see what reception this PR gets. Hopefully much of what you have done here can be re-used in other views.

Don't forget to implement an option (default = off) to show previews.

Thanks. Any tips on how to add the option? My latest commit is everything I did this morning, but still got nowhere :\

shiruken1 avatar Aug 13 '25 20:08 shiruken1

@shiruken1 I would envisage another switch in the settings menu in the headerbar under "Show in View". I'll see if I can implement if you like. Looks like this is what you started anyway - not immediately obvious what is broken from eyeballing the code.

It looks like you have made some code style corrections relating to code that would not otherwise be touched? I would advise against this as it just adds noise to the diff. These unrelated changes should be done in a separate cleanup/Gtk4 prep PR.

jeremypw avatar Aug 14 '25 11:08 jeremypw

You also need to listen for changes to the setting and update the ColumnView accordingly, adding or removing the details pane as appropriate.

jeremypw avatar Aug 14 '25 11:08 jeremypw

Don't forget to de-lint:

jeremy@OS8-VM-2:~/Elementary/files/build$ lint
../src/View/AbstractDirectoryView.vala
 1451.64    error   Expected space before paren                     space-before-paren
../src/View/Window.vala
 1102.47    error   Expected a single space                         double-spaces
../src/View/Miller.vala
  132.64    error   Expected space before paren                     space-before-paren

3 errors, 0 warnings

jeremypw avatar Aug 14 '25 11:08 jeremypw

Thanks for the feedback and the reminder to lint!

not immediately obvious what is broken from eyeballing the code.

Yea. I get this despite all I've written :weary:

Screenshot from 2025-08-14 06-57-53

shiruken1 avatar Aug 14 '25 12:08 shiruken1

Yea. I get this despite all I've written 😩

You need to actually install the branch because you have changed the schema. Even when running in ./build it will still use system settings. Same thing applies if you ever want to work on a plugin. You have to install to test.

jeremypw avatar Aug 14 '25 15:08 jeremypw

Yea. I get this despite all I've written 😩

You need to actually install the branch because you have changed the schema. Even when running in ./build it will still use system settings. Same thing applies if you ever want to work on a plugin. You have to install to test.

Ah! Thanks for the tip

shiruken1 avatar Aug 15 '25 12:08 shiruken1

@jeremypw :face_with_peeking_eye: how do I tell the install to build/install/bundle libcore? I'm getting "io.elementary.files: symbol lookup error: io.elementary.files: undefined symbol: files_file_is_text" (I added File.is_text ())

shiruken1 avatar Aug 16 '25 15:08 shiruken1

@shiruken1 You should not need to do anything in particular - just run sudo ninja install. Make sure that that there aren't any old Files processes running though using System Monitor. Try deleting and recreating the build directory to make sure everything is being rebuilt if you still have trouble.

jeremypw avatar Aug 16 '25 16:08 jeremypw

I have a couple of aliases in my .bashrc file to save typing:

# Build new meson project (run in root)
alias build='meson setup build --prefix=/usr && cd ./build && ninja'
# Rebuild existing meson project (run in /build)
alias rebuild='cd .. && sudo rm -R ./build && build'

jeremypw avatar Aug 16 '25 17:08 jeremypw

@jeremypw I'm really sorry, total noob question: how do I get warnings on the console? I added --buildtype=debug to meson, I tailed /var/log/syslog, I grepped for "log level" in the codebase, I'm lost :flushed: TIA!

shiruken1 avatar Aug 17 '25 12:08 shiruken1

@shiruken1 Which warnings do you want? Messages from Files at or above the log level "warning" are automatically sent to stderr so just launch Files in the Terminal with io.elementary.files to see those. To see "debug" level messages and above launch in the Terminal with G_MESSAGE_DEBUG=all io.elementary.files.

You can also temporarily insert your own messages with warning ("insert-your-message") in the code (I do this a lot when debugging"). Or change an existing debug() message to warning ().

I've only ever used the default buildtype for meson. What problem are you trying to solve?

jeremypw avatar Aug 17 '25 13:08 jeremypw

@jeremypw I'm trying to see if my on_show_file_preview_changed is even being executed. I added a warning to my draw_file_details and clear_file_details functions, but nothing on the terminal, despite them obviously working.

I tried G_MESSAGE_DEBUG=all io.elementary.files and also nothing. Debugging is hard without logs :weary:

P.S: when I don't install, warnings work just fine, but when I click the File Preview option I get a Seg Fault with nothing to go on

shiruken1 avatar Aug 18 '25 11:08 shiruken1

It may be G_MESSAGES_DEBUG with an S try that?

vjr avatar Aug 18 '25 11:08 vjr