Add preview column to Column View
Fixes #1601 and gives text preview for "text/*" and "application/sql" mimetypes
@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
@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 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.
@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!
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
All set. I went ahead and fixed the line-length warnings in the files I worked with. Hope that's ok.
@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 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
@shiruken1 You did remarkably well for your first attempt with Vala/elementaryos!
@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 :(
P.S: no, SQL is not code... it's hell :sob:
@jeremypw All set! :tada:
I'm thinking halign = CENTER looks better :hugs:
Buttons are usually aligned at start or end when on the last line of a dialog but @danirabbit has final say on design.
@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?
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.
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 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.
You also need to listen for changes to the setting and update the ColumnView accordingly, adding or removing the details pane as appropriate.
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
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:
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.
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
./buildit 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
@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 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.
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 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 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 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
It may be G_MESSAGES_DEBUG with an S try that?