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

Improve Files section #364

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LucyJimenez
Copy link
Contributor

This PR includes updates to improve clarity and accuracy in the documentation for the Files section:

  • File Associations Path: The path to access the 'File Associations' tab has been updated for better clarity.
  • Options Menu Filter Name: Corrected the filter name in the 'Options Menu' section for accuracy.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

I apologize for the long delay in reviewing this, given my cat, the trip and my friends and family visiting, it fell through the cracks of my GH notifications and task list. Please feel free to bug me on Discord and/or remind me at meetings (though I'll try to do a better job at keeping up going forward).

Unfortunately, it seems this wasn't based on the upstream master branch and instead has a bunch of commits from your previous PR, which you can see in the commit history and in the Files changed tab. However, a minor bit of Git surgery will resolve this; either I can take care of it for you, or you can run the following commands:

git switch update_doc
git reset --hard master
git cherry-pick c640ca9efc2c6aa3b4236b81d37edac6b7fc8fb2
git push --force --set-upstream origin update_doc

(The --set-upstream origin update_doc is included in case you haven't already set this as your upstream tracking branch)

I also have a couple comments on the intended changes themselves, but I'll hold them till after fixed to avoid any hiccups resolving this issue. Thanks!

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Per our discussion, I went ahead with implementing the "Git surgery" to fix the branch history issue identified. If you want to work on this branch further locally (instead of just on GitHub via creating or accepting suggestions), you'll need to run the following in your repo before working on it to pull down the updated version (since I had to inherently rewrite Git history to fix the problem):

git switch update_doc
git fetch --all
git reset --hard origin/update_doc

Besides that, as mentioned I did have some feedback on one of the changes, which I made as a review comment. Thanks!

@@ -81,7 +81,7 @@ File associations
=================

:guilabel:`Files` allows you to associate different external applications with specific file extensions they can open.
Under the :guilabel:`File associations` tab of the :guilabel:`Files` preferences pane, you can add file types and set the external program used to open each of them by default.
Under the :guilabel:`Preferences --> File associations` tab of the :guilabel:`Files` preferences pane, you can add file types and set the external program used to open each of them by default.
Copy link
Member

@CAM-Gerlach CAM-Gerlach Apr 22, 2024

Choose a reason for hiding this comment

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

I'm unclear on the motivation for this change was made, since it implies two conflicting potential interpretations, neither of which is correct (unlike the previous text):

  • As mentioned, the existing text already states that this is the File associations tab of the Files preferences pane, so its confusing why "Preferences" is repeated again here
  • On first reading, this change implies that File associations is a pane underneath preferences, but that's not the case as mentioned
  • Furthermore, in context as written, it alternatively implies there is a Preferences in the Files preferences pane (and underneath that a File associations tab), but that is of course not correct either
  • Additionally, :guilabel: is for single, specific GUI labels; to support multiple levels of things you'd need :menuselection:

Without more context on the desired intent here, I can propose either a revert to the existing, correct text which explicitly specifies that this is a tab of a specific pane under preferences:

Suggested change
Under the :guilabel:`Preferences --> File associations` tab of the :guilabel:`Files` preferences pane, you can add file types and set the external program used to open each of them by default.
Under the :guilabel:`File associations` tab of the :guilabel:`Files` preferences pane, you can add file types and set the external program used to open each of them by default.

Or, we could specify the complete path all in one go, which is more direct and concise but lacks explicit cues for the reader as to what they should be looking for at each level, and also runs into the issue you raised in #365 which will take some engineering work to solve, and is de-facto gated by the implementation of the new theme, so I don't recommend this for now:

Suggested change
Under the :guilabel:`Preferences --> File associations` tab of the :guilabel:`Files` preferences pane, you can add file types and set the external program used to open each of them by default.
Under :menuselection:`Preferences --> Files --> File associations`, you can add file types and set the external program used to open each of them by default.

If you could help me understand better the problem this is intended to solve, I could try to help propose a better solution. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants