From d4ca29ed2c9f0ec9a803013fd915ae705dfe38af Mon Sep 17 00:00:00 2001 From: Jonathan Wren Date: Sat, 2 Jan 2021 12:24:21 -0800 Subject: [PATCH 01/10] Fix broken search bar in docs site (#1135) * Fixes for search on docs site We previously didn't include the search results page in our CI testing, so we missed some issues on that page. This ensures that page is part of regular testing, and also includes fixes for the issues present. * fix sidebar contrast --- .github/workflows/docs.yaml | 2 +- docs_theme/assets/colors.css | 1 + docs_theme/assets/theme.css | 56 ++++++++++++++++++++++++++++++------ docs_theme/main.html | 16 +++++++---- docs_theme/search.html | 29 +++++++++++++++++++ 5 files changed, 88 insertions(+), 16 deletions(-) create mode 100644 docs_theme/search.html diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 6208ad5e..3b8cfa96 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -61,7 +61,7 @@ jobs: env: site_url: http://127.0.0.1:8000 run: | - select="{urls: [\"${site_url}/\", .urlset.url[].loc]}" + select="{urls: [\"${site_url}/\", \"${site_url}/search.html?q=jrnl\", .urlset.url[].loc]}" curl -s "$site_url/sitemap.xml" | poetry run xq "$select" > list.json - name: Accessibility testing (Pa11y) diff --git a/docs_theme/assets/colors.css b/docs_theme/assets/colors.css index 1a518461..93d84b4d 100644 --- a/docs_theme/assets/colors.css +++ b/docs_theme/assets/colors.css @@ -15,6 +15,7 @@ --yellow: #e2b93d; /* For light bg */ + --black: #404040; --teal: #2a8068; --dark-blue: #356eb7; --mid-purple: #846392; diff --git a/docs_theme/assets/theme.css b/docs_theme/assets/theme.css index a98c49bc..491a0baa 100644 --- a/docs_theme/assets/theme.css +++ b/docs_theme/assets/theme.css @@ -8,6 +8,7 @@ body.wy-body-for-nav, section.wy-nav-content-wrap { background-color: var(--white); + color: var(--black); } .rst-content pre { @@ -102,7 +103,7 @@ a.icon-home:before { .wy-menu-vertical a, .wy-menu-vertical li ul li a { font-size: 16px; - color: var(--off-white); + color: var(--white); line-height: 2em; } @@ -167,7 +168,7 @@ a.icon-home:before { } .wy-menu-vertical li a { - color: var(--off-white) !important; + color: var(--white) !important; font-weight: 300; } @@ -185,18 +186,20 @@ footer { } .wy-side-nav-search input[type=text], +.mkdocs-search input[type=text], form .search-query { - background-color: var(--black-shadow) !important; + background-color: var(--off-white); border: none; - box-shadow: none; margin-bottom: 1em; - color: var(--white); + color: var(--black); font-weight: 500; + box-shadow: none; } .wy-side-nav-search input[type=text]::placeholder, +.mkdocs-search input[type=text]::placeholder, form .search-query::placeholder { - color: var(--off-white); + color: var(--dark-purple); } .wy-side-nav-search > a:hover { @@ -298,19 +301,54 @@ ol>li:before { margin-top: 20px; } -.wy-side-nav-search input[type="text"] { +.wy-side-nav-search input[type="text"], +.mkdocs-search input[type=text] { border-radius: 50px 0 0 50px; height: 32px; border-right: none; + margin: 0; } .mkdocs-search button { - background-color: var(--black-shadow); + background-color: var(--off-white); border: none; box-shadow: none; - color: var(--white); + color: var(--mid-purple); border-radius: 0 50px 50px 0; height: 32px; width: 2.5em; overflow: hidden; } + +.mkdocs-search { + border-radius: 50px; +} + +.mkdocs-search:focus-within { + box-shadow: 0 2px 25px 0 var(--blacker-shadow); + transition: all .5s ease; +} + +.rst-content div[role="main"] .mkdocs-search input[type="text"] { + border-right: none; + font-size: 100%; + height: 48px; + margin: 0; +} + +.rst-content div[role="main"] .mkdocs-search button { + border-left: none; + font-size: 100%; + height: 48px; +} + +.rst-content div[role="main"] .mkdocs-search button:before { + font-size: 140%; + position: relative; + left: -7px; + top: -1px; +} + +.search-results { + margin-top: 0; +} diff --git a/docs_theme/main.html b/docs_theme/main.html index 7d18ba8d..18a4f1cf 100644 --- a/docs_theme/main.html +++ b/docs_theme/main.html @@ -1,8 +1,12 @@ {% extends "base.html" %} -{% block search_button %} -
- - -
-{% endblock %} +{%- block search_button %} + {% if 'search' in config['plugins'] %} +
+ +
+ {% endif %} +{%- endblock %} diff --git a/docs_theme/search.html b/docs_theme/search.html new file mode 100644 index 00000000..b191fc2a --- /dev/null +++ b/docs_theme/search.html @@ -0,0 +1,29 @@ +{% extends "main.html" %} + +{% block content %} + +
+
+ + + + +
+
+ +

Results

+ +
+ Searching... +
+ +{% endblock %} From ada73939791bded7a67b331e70d6dd61f672d8eb Mon Sep 17 00:00:00 2001 From: Jrnl Bot Date: Sat, 2 Jan 2021 20:27:26 +0000 Subject: [PATCH 02/10] Update changelog [ci skip] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc43f372..0b0143b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ **Documentation:** +- Fix broken search bar in docs site [\#1135](https://github.com/jrnl-org/jrnl/pull/1135) ([wren](https://github.com/wren)) - Fix search on docs site [\#1133](https://github.com/jrnl-org/jrnl/pull/1133) ([wren](https://github.com/wren)) - Add packaging label to changelog generator config [\#1132](https://github.com/jrnl-org/jrnl/pull/1132) ([wren](https://github.com/wren)) - Fix failing contrast test in accessibility tools on docs site [\#1126](https://github.com/jrnl-org/jrnl/pull/1126) ([wren](https://github.com/wren)) From c47c1e209e544dd4d41b82d8a2217fd8b4ea3f40 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 2 Jan 2021 12:28:03 -0800 Subject: [PATCH 03/10] Bump keyring from 21.7.0 to 21.8.0 (#1136) Bumps [keyring](https://github.com/jaraco/keyring) from 21.7.0 to 21.8.0. - [Release notes](https://github.com/jaraco/keyring/releases) - [Changelog](https://github.com/jaraco/keyring/blob/main/CHANGES.rst) - [Commits](https://github.com/jaraco/keyring/compare/v21.7.0...v21.8.0) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/poetry.lock b/poetry.lock index cd5ac6d2..fc0f5895 100644 --- a/poetry.lock +++ b/poetry.lock @@ -212,7 +212,7 @@ python-versions = ">=3.6" [[package]] name = "keyring" -version = "21.7.0" +version = "21.8.0" description = "Store and access your passwords safely." category = "main" optional = false @@ -225,7 +225,7 @@ pywin32-ctypes = {version = "<0.1.0 || >0.1.0,<0.1.1 || >0.1.1", markers = "sys_ SecretStorage = {version = ">=3.2", markers = "sys_platform == \"linux\""} [package.extras] -docs = ["sphinx", "jaraco.packaging (>=3.2)", "rst.linker (>=1.9)"] +docs = ["sphinx", "jaraco.packaging (>=8.2)", "rst.linker (>=1.9)"] testing = ["pytest (>=3.5,!=3.7.3)", "pytest-checkdocs (>=1.2.3)", "pytest-flake8", "pytest-cov", "jaraco.test (>=3.2.0)", "pytest-black (>=0.3.7)", "pytest-mypy"] [[package]] @@ -735,8 +735,8 @@ joblib = [ {file = "joblib-0.17.0.tar.gz", hash = "sha256:9e284edd6be6b71883a63c9b7f124738a3c16195513ad940eae7e3438de885d5"}, ] keyring = [ - {file = "keyring-21.7.0-py3-none-any.whl", hash = "sha256:4c41ce4f6d1ee91d589a346699ef5a94ba3429603ac8f700cc0097644cdd6748"}, - {file = "keyring-21.7.0.tar.gz", hash = "sha256:a144f7e1044c897c3976202af868cb0ac860f4d433d5d0f8e750fa1a2f0f0b50"}, + {file = "keyring-21.8.0-py3-none-any.whl", hash = "sha256:4be9cbaaaf83e61d6399f733d113ede7d1c73bc75cb6aeb64eee0f6ac39b30ea"}, + {file = "keyring-21.8.0.tar.gz", hash = "sha256:1746d3ac913d449a090caf11e9e4af00e26c3f7f7e81027872192b2398b98675"}, ] livereload = [ {file = "livereload-2.6.3.tar.gz", hash = "sha256:776f2f865e59fde56490a56bcc6773b6917366bce0c267c60ee8aaf1a0959869"}, From e9fd5cdc0eef8d881c401825bfe04ce071220555 Mon Sep 17 00:00:00 2001 From: Karim Rahal Date: Sat, 2 Jan 2021 23:23:15 +0200 Subject: [PATCH 04/10] Allow custom extensions when editing (for easier syntax highlighting) (#1139) * Create tests and steps for temporary filename suffix * Have temporary filename suffix be -{template_filename} or .jrnl * Finalize extension_editor_file * Make suffix local variable in get_text_from_editor --- .../configs/editor_markdown_extension.yaml | 18 ++++++++++++++++++ features/data/templates/extension.md | 0 features/file_storage.feature | 10 ++++++++++ features/steps/core.py | 8 ++++++++ jrnl/editor.py | 7 ++++++- 5 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 features/data/configs/editor_markdown_extension.yaml create mode 100644 features/data/templates/extension.md diff --git a/features/data/configs/editor_markdown_extension.yaml b/features/data/configs/editor_markdown_extension.yaml new file mode 100644 index 00000000..bf3b8d8e --- /dev/null +++ b/features/data/configs/editor_markdown_extension.yaml @@ -0,0 +1,18 @@ +default_hour: 9 +default_minute: 0 +editor: "" +encrypt: false +highlight: true +editor: "vim" +journals: + default: features/journals/editor_markdown_extension.journal +linewrap: 80 +tagsymbols: "@" +template: features/templates/extension.md +timeformat: "%Y-%m-%d %H:%M" +indent_character: "|" +colors: + date: none + title: none + body: none + tags: none diff --git a/features/data/templates/extension.md b/features/data/templates/extension.md new file mode 100644 index 00000000..e69de29b diff --git a/features/file_storage.feature b/features/file_storage.feature index 0e2c0b3c..33619365 100644 --- a/features/file_storage.feature +++ b/features/file_storage.feature @@ -44,3 +44,13 @@ Feature: Journals iteracting with the file system in a way that users can see And we change directory to "features" And we run "jrnl -n 1" Then the output should contain "hello world" + + Scenario: the temporary filename suffix should default to ".jrnl" + Given we use the config "editor.yaml" + When we run "jrnl --edit" + Then the temporary filename suffix should be ".jrnl" + + Scenario: the temporary filename suffix should be "-{template_filename}" + Given we use the config "editor_markdown_extension.yaml" + When we run "jrnl --edit" + Then the temporary filename suffix should be "-extension.md" diff --git a/features/steps/core.py b/features/steps/core.py index d579b6d2..af22c0cd 100644 --- a/features/steps/core.py +++ b/features/steps/core.py @@ -248,6 +248,14 @@ def contains_editor_file(context, method, text=""): assert False, f"Method '{method}' not supported" +@then('the temporary filename suffix should be "{suffix}"') +def extension_editor_file(context, suffix): + filename = Path(context.editor_file["name"]).name + delimiter = "-" if "-" in filename else "." + filename_suffix = delimiter + filename.split(delimiter)[-1] + assert filename_suffix == suffix + + def _mock_getpass(inputs): def prompt_return(prompt=""): if type(inputs) == str: diff --git a/jrnl/editor.py b/jrnl/editor.py index 3397cdac..1a68028d 100644 --- a/jrnl/editor.py +++ b/jrnl/editor.py @@ -5,6 +5,7 @@ import subprocess import sys import tempfile import textwrap +from pathlib import Path from .color import ERROR_COLOR from .color import RESET_COLOR @@ -12,7 +13,11 @@ from .os_compat import on_windows def get_text_from_editor(config, template=""): - filehandle, tmpfile = tempfile.mkstemp(prefix="jrnl", text=True, suffix=".txt") + suffix = ".jrnl" + if config["template"]: + template_filename = Path(config["template"]).name + suffix = "-" + template_filename + filehandle, tmpfile = tempfile.mkstemp(prefix="jrnl", text=True, suffix=suffix) os.close(filehandle) with open(tmpfile, "w", encoding="utf-8") as f: From 9f1bef7fdef3b9198a88ebe6cb46ec72f27d9eb6 Mon Sep 17 00:00:00 2001 From: Jrnl Bot Date: Sat, 2 Jan 2021 21:24:55 +0000 Subject: [PATCH 05/10] Update changelog [ci skip] --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b0143b4..84e79207 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ **Implemented enhancements:** - Implement dependency tracker/updater [\#1120](https://github.com/jrnl-org/jrnl/issues/1120) +- Change temporary file names for better text editor integration [\#1080](https://github.com/jrnl-org/jrnl/issues/1080) +- Allow custom file extension for `jrnl --edit` command [\#1059](https://github.com/jrnl-org/jrnl/issues/1059) +- Allow custom extensions when editing \(for easier syntax highlighting\) [\#1139](https://github.com/jrnl-org/jrnl/pull/1139) ([KarimPwnz](https://github.com/KarimPwnz)) **Build:** From 57de4f9ba44eb4be9ca31814bab4e4772d136a3c Mon Sep 17 00:00:00 2001 From: Karim Rahal Date: Sat, 2 Jan 2021 23:26:45 +0200 Subject: [PATCH 06/10] Fix keyring error handling (#1138) * Capture KeyringLocked exception and allow manual password entry * Create LockedKeyring for steps * Support types in set_keyring step * Clarify LockedKeyring docs * Deal with locked exceptions elsewhere too * Create behave tests for locked keyring * Fix linting * Fix keyring step to allow no type * Handle all keyring retrieval errors * Better keyring error handling * Remove locked keyring for steps; generalize failed keyring * Finalize tests for keyring handling * Update set_keyring step * Make password of failed keyring encryption test more semantic --- features/password.feature | 39 ++++++++++++++++++++++++++++++++++----- features/steps/core.py | 18 ++++++++++-------- jrnl/EncryptedJournal.py | 19 ++++++++++++------- 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/features/password.feature b/features/password.feature index 86fa7f6b..332ba86e 100644 --- a/features/password.feature +++ b/features/password.feature @@ -24,6 +24,7 @@ Feature: Using the installed keyring n """ Then we should get no error + And we should not see the message "Failed to retrieve keyring" Scenario: Encrypt journal with no keyring backend and do store in keyring Given we use the config "simple.yaml" @@ -36,25 +37,53 @@ Feature: Using the installed keyring y """ Then we should get no error + And we should not see the message "Failed to retrieve keyring" # @todo add step to check contents of keyring @todo Scenario: Open an encrypted journal with wrong password in keyring # This should ask the user for the password after the keyring fails - @todo - Scenario: Open encrypted journal when keyring exists but fails - # This should ask the user for the password after the keyring fails - @todo Scenario: Decrypt journal with password in keyring @todo Scenario: Decrypt journal without a keyring - @todo + Scenario: Encrypt journal when keyring exists but fails + Given we use the config "simple.yaml" + And we have a failed keyring + When we run "jrnl --encrypt" and enter + """ + this password will not be saved in keyring + this password will not be saved in keyring + y + """ + Then we should see the message "Failed to retrieve keyring" + And we should get no error + And we should be prompted for a password + And the config for journal "default" should have "encrypt" set to "bool:True" + Scenario: Decrypt journal when keyring exists but fails + Given we use the config "encrypted.yaml" + And we have a failed keyring + When we run "jrnl --decrypt" and enter "bad doggie no biscuit" + Then we should see the message "Failed to retrieve keyring" + And we should get no error + And we should be prompted for a password + And we should see the message "Journal decrypted" + And the config for journal "default" should have "encrypt" set to "bool:False" + And the journal should have 2 entries + + Scenario: Open encrypted journal when keyring exists but fails # This should ask the user for the password after the keyring fails + Given we use the config "encrypted.yaml" + And we have a failed keyring + When we run "jrnl -n 1" and enter "bad doggie no biscuit" + Then we should see the message "Failed to retrieve keyring" + And we should get no error + And we should be prompted for a password + And the output should contain "2013-06-10 15:40 Life is good" Scenario: Mistyping your password Given we use the config "simple.yaml" diff --git a/features/steps/core.py b/features/steps/core.py index af22c0cd..2f72a473 100644 --- a/features/steps/core.py +++ b/features/steps/core.py @@ -69,21 +69,19 @@ class NoKeyring(keyring.backend.KeyringBackend): class FailedKeyring(keyring.backend.KeyringBackend): """ - A keyring that simulates an environment with a keyring that has passwords, but fails - to return them. + A keyring that cannot be retrieved. """ priority = 2 - keys = defaultdict(dict) def set_password(self, servicename, username, password): - self.keys[servicename][username] = password + raise keyring.errors.KeyringError def get_password(self, servicename, username): - raise keyring.errors.NoKeyringError + raise keyring.errors.KeyringError def delete_password(self, servicename, username): - self.keys[servicename][username] = None + raise keyring.errors.KeyringError # set a default keyring @@ -148,8 +146,12 @@ def use_password(context, password, num=1): @given("we have a keyring") -def set_keyring(context): - keyring.set_keyring(TestKeyring()) +@given("we have a {type} keyring") +def set_keyring(context, type=""): + if type == "failed": + keyring.set_keyring(FailedKeyring()) + else: + keyring.set_keyring(TestKeyring()) @given("we do not have a keyring") diff --git a/jrnl/EncryptedJournal.py b/jrnl/EncryptedJournal.py index e1d248aa..7354e7a2 100644 --- a/jrnl/EncryptedJournal.py +++ b/jrnl/EncryptedJournal.py @@ -176,7 +176,9 @@ def get_keychain(journal_name): try: return keyring.get_password("jrnl", journal_name) - except RuntimeError: + except keyring.errors.KeyringError as e: + if not isinstance(e, keyring.errors.NoKeyringError): + print("Failed to retrieve keyring", file=sys.stderr) return "" @@ -186,13 +188,16 @@ def set_keychain(journal_name, password): if password is None: try: keyring.delete_password("jrnl", journal_name) - except keyring.errors.PasswordDeleteError: + except keyring.errors.KeyringError: pass else: try: keyring.set_password("jrnl", journal_name, password) - except keyring.errors.NoKeyringError: - print( - "Keyring backend not found. Please install one of the supported backends by visiting: https://pypi.org/project/keyring/", - file=sys.stderr, - ) + except keyring.errors.KeyringError as e: + if isinstance(e, keyring.errors.NoKeyringError): + print( + "Keyring backend not found. Please install one of the supported backends by visiting: https://pypi.org/project/keyring/", + file=sys.stderr, + ) + else: + print("Failed to retrieve keyring", file=sys.stderr) From bddec64faa5584a69f2f498439cb2cb592d27067 Mon Sep 17 00:00:00 2001 From: Jrnl Bot Date: Sat, 2 Jan 2021 21:28:22 +0000 Subject: [PATCH 07/10] Update changelog [ci skip] --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84e79207..dd2e3f38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ - Allow custom file extension for `jrnl --edit` command [\#1059](https://github.com/jrnl-org/jrnl/issues/1059) - Allow custom extensions when editing \(for easier syntax highlighting\) [\#1139](https://github.com/jrnl-org/jrnl/pull/1139) ([KarimPwnz](https://github.com/KarimPwnz)) +**Fixed bugs:** + +- Error if password exists in keyring, but retrieval fails for any reason [\#1020](https://github.com/jrnl-org/jrnl/issues/1020) +- Fix keyring error handling [\#1138](https://github.com/jrnl-org/jrnl/pull/1138) ([KarimPwnz](https://github.com/KarimPwnz)) + **Build:** - Fix changelog generator [\#1127](https://github.com/jrnl-org/jrnl/pull/1127) ([wren](https://github.com/wren)) From f0414025339eb6cd5e882c7881dd2e12c88b466f Mon Sep 17 00:00:00 2001 From: Jonathan Wren Date: Sat, 2 Jan 2021 14:53:54 -0800 Subject: [PATCH 08/10] Add packaging label to included sections in changelog (#1140) --- .github/workflows/changelog.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/changelog.yaml b/.github/workflows/changelog.yaml index 23177052..dc52daed 100644 --- a/.github/workflows/changelog.yaml +++ b/.github/workflows/changelog.yaml @@ -104,7 +104,7 @@ jobs: issuesWoLabels: false unreleased: true compareLink: true - includeLabels: bug,enhancement,documentation,build,deprecated + includeLabels: bug,enhancement,documentation,build,packaging,deprecated excludeLabels: stale,wontfix excludeTagsRegex: ${{ env.TAG_REGEX }} sinceTag: ${{ env.SINCE_TAG }} From e0c53c28bbd2cbefe0a1ecfec3e8319b6bfab1a1 Mon Sep 17 00:00:00 2001 From: Jrnl Bot Date: Sat, 2 Jan 2021 22:55:45 +0000 Subject: [PATCH 09/10] Update changelog [ci skip] --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd2e3f38..a75baaf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,13 @@ - Add packaging label to changelog generator config [\#1132](https://github.com/jrnl-org/jrnl/pull/1132) ([wren](https://github.com/wren)) - Fix failing contrast test in accessibility tools on docs site [\#1126](https://github.com/jrnl-org/jrnl/pull/1126) ([wren](https://github.com/wren)) +**Packaging:** + +- Bump keyring from 21.7.0 to 21.8.0 [\#1136](https://github.com/jrnl-org/jrnl/pull/1136) ([dependabot[bot]](https://github.com/apps/dependabot)) +- Bump pytz from 2020.4 to 2020.5 [\#1130](https://github.com/jrnl-org/jrnl/pull/1130) ([dependabot-preview[bot]](https://github.com/apps/dependabot-preview)) +- Bump pytest from 6.2.0 to 6.2.1 [\#1129](https://github.com/jrnl-org/jrnl/pull/1129) ([dependabot-preview[bot]](https://github.com/apps/dependabot-preview)) +- Bump keyring from 21.5.0 to 21.7.0 [\#1128](https://github.com/jrnl-org/jrnl/pull/1128) ([dependabot-preview[bot]](https://github.com/apps/dependabot-preview)) + ## [v2.6](https://pypi.org/project/jrnl/v2.6/) (2020-12-20) [Full Changelog](https://github.com/jrnl-org/jrnl/compare/v2.5...v2.6) From c155bafa848445531ac01f60422e95c59111dd16 Mon Sep 17 00:00:00 2001 From: Micah Jerome Ellison Date: Sat, 2 Jan 2021 15:19:44 -0800 Subject: [PATCH 10/10] Notify user when config directory can't be created because there is already a file with the same name (#1134) * Moving configuration values and methods from install.py to config.py -- everything is broken right now * Using context to store config path - still lots broken * Use mocks and context.config_path to store test configs - many tests still broken though * Update changelog [ci skip] * Fix jrnl --ls crash * Fix crash when no editor configured * Attempt to patch config path with test data - doesn't appear to be working * Properly use patched config path and add config given to scenario that wasn't using it * Fix copypasta * Fix editor test that needed patched config and trapping for system exit * Add exception and handling for when configuration directory is actually a file * Remove extraneous comment * Use more generic JrnlError with messaging switchboard * Format code a bit nicer * Remove unnecessary given in diagnostic test * Ensure full error message is output * Remove unnecessary whitespace characters --- features/steps/core.py | 77 +++++++++++++++++++++++------------------ jrnl/cli.py | 5 +++ jrnl/config.py | 70 ++++++++++++++++++++++++++++++++++--- jrnl/exception.py | 26 ++++++++++++++ jrnl/install.py | 69 ++++++++---------------------------- jrnl/jrnl.py | 3 +- jrnl/output.py | 10 +++--- tests/test_exception.py | 19 ++++++++++ 8 files changed, 182 insertions(+), 97 deletions(-) create mode 100644 tests/test_exception.py diff --git a/features/steps/core.py b/features/steps/core.py index 2f72a473..e3af8243 100644 --- a/features/steps/core.py +++ b/features/steps/core.py @@ -19,7 +19,6 @@ import yaml from jrnl import Journal from jrnl import __version__ -from jrnl import install from jrnl import plugins from jrnl.cli import cli from jrnl.config import load_config @@ -92,25 +91,25 @@ def ushlex(command): return shlex.split(command, posix=not on_windows) -def read_journal(journal_name="default"): - config = load_config(install.CONFIG_FILE_PATH) - with open(config["journals"][journal_name]) as journal_file: +def read_journal(context, journal_name="default"): + configuration = load_config(context.config_path) + with open(configuration["journals"][journal_name]) as journal_file: journal = journal_file.read() return journal -def open_journal(journal_name="default"): - config = load_config(install.CONFIG_FILE_PATH) - journal_conf = config["journals"][journal_name] +def open_journal(context, journal_name="default"): + configuration = load_config(context.config_path) + journal_conf = configuration["journals"][journal_name] # We can override the default config on a by-journal basis if type(journal_conf) is dict: - config.update(journal_conf) + configuration.update(journal_conf) # But also just give them a string to point to the journal file else: - config["journal"] = journal_conf + configuration["journal"] = journal_conf - return Journal.open_journal(journal_name, config) + return Journal.open_journal(journal_name, configuration) def read_value_from_string(string): @@ -128,10 +127,11 @@ def read_value_from_string(string): def set_config(context, config_file): full_path = os.path.join("features/configs", config_file) - install.CONFIG_FILE_PATH = os.path.abspath(full_path) + context.config_path = os.path.abspath(full_path) + if config_file.endswith("yaml") and os.path.exists(full_path): # Add jrnl version to file for 2.x journals - with open(install.CONFIG_FILE_PATH, "a") as cf: + with open(context.config_path, "a") as cf: cf.write("version: {}".format(__version__)) @@ -196,11 +196,18 @@ def open_editor_and_enter(context, method, text=""): with \ patch("subprocess.call", side_effect=_mock_editor) as mock_editor, \ patch("getpass.getpass", side_effect=_mock_getpass(password)) as mock_getpass, \ - patch("sys.stdin.isatty", return_value=True) \ + patch("sys.stdin.isatty", return_value=True), \ + patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ + patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ : context.editor = mock_editor context.getpass = mock_getpass - cli(["--edit"]) + try: + cli(["--edit"]) + context.exit_status = 0 + except SystemExit as e: + context.exit_status = e.code + # fmt: on @@ -314,7 +321,9 @@ def run_with_input(context, command, inputs=""): patch("builtins.input", side_effect=_mock_input(text)) as mock_input, \ patch("getpass.getpass", side_effect=_mock_getpass(password)) as mock_getpass, \ patch("sys.stdin.read", side_effect=text) as mock_read, \ - patch("subprocess.call", side_effect=_mock_editor) as mock_editor \ + patch("subprocess.call", side_effect=_mock_editor) as mock_editor, \ + patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ + patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ : try: cli(args or []) @@ -396,7 +405,9 @@ def run(context, command, text=""): patch("sys.argv", args), \ patch("getpass.getpass", side_effect=_mock_getpass(password)) as mock_getpass, \ patch("subprocess.call", side_effect=_mock_editor) as mock_editor, \ - patch("sys.stdin.read", side_effect=lambda: text) \ + patch("sys.stdin.read", side_effect=lambda: text), \ + patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ + patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ : context.editor = mock_editor context.getpass = mock_getpass @@ -544,32 +555,32 @@ def check_not_message(context, text): @then('the journal should contain "{text}"') @then('journal "{journal_name}" should contain "{text}"') def check_journal_content(context, text, journal_name="default"): - journal = read_journal(journal_name) + journal = read_journal(context, journal_name) assert text in journal, journal @then('the journal should not contain "{text}"') @then('journal "{journal_name}" should not contain "{text}"') def check_not_journal_content(context, text, journal_name="default"): - journal = read_journal(journal_name) + journal = read_journal(context, journal_name) assert text not in journal, journal @then("the journal should not exist") @then('journal "{journal_name}" should not exist') def journal_doesnt_exist(context, journal_name="default"): - config = load_config(install.CONFIG_FILE_PATH) + configuration = load_config(context.config_path) - journal_path = config["journals"][journal_name] + journal_path = configuration["journals"][journal_name] assert not os.path.exists(journal_path) @then("the journal should exist") @then('journal "{journal_name}" should exist') def journal_exists(context, journal_name="default"): - config = load_config(install.CONFIG_FILE_PATH) + configuration = load_config(context.config_path) - journal_path = config["journals"][journal_name] + journal_path = configuration["journals"][journal_name] assert os.path.exists(journal_path) @@ -578,23 +589,23 @@ def journal_exists(context, journal_name="default"): @then('the config for journal "{journal}" should have "{key}" set to "{value}"') def config_var(context, key, value="", journal=None): value = read_value_from_string(value or context.text or "") - config = load_config(install.CONFIG_FILE_PATH) + configuration = load_config(context.config_path) if journal: - config = config["journals"][journal] + configuration = configuration["journals"][journal] - assert key in config - assert config[key] == value + assert key in configuration + assert configuration[key] == value @then('the config for journal "{journal}" should not have "{key}" set') def config_no_var(context, key, value="", journal=None): - config = load_config(install.CONFIG_FILE_PATH) + configuration = load_config(context.config_path) if journal: - config = config["journals"][journal] + configuration = configuration["journals"][journal] - assert key not in config + assert key not in configuration @then("the journal should have {number:d} entries") @@ -602,15 +613,15 @@ def config_no_var(context, key, value="", journal=None): @then('journal "{journal_name}" should have {number:d} entries') @then('journal "{journal_name}" should have {number:d} entry') def check_journal_entries(context, number, journal_name="default"): - journal = open_journal(journal_name) + journal = open_journal(context, journal_name) assert len(journal.entries) == number @when("the journal directory is listed") def list_journal_directory(context, journal="default"): - with open(install.CONFIG_FILE_PATH) as config_file: - config = yaml.load(config_file, Loader=yaml.FullLoader) - journal_path = config["journals"][journal] + with open(context.config_path) as config_file: + configuration = yaml.load(config_file, Loader=yaml.FullLoader) + journal_path = configuration["journals"][journal] for root, dirnames, f in os.walk(journal_path): for file in f: print(os.path.join(root, file)) diff --git a/jrnl/cli.py b/jrnl/cli.py index e010f38e..93a7e899 100644 --- a/jrnl/cli.py +++ b/jrnl/cli.py @@ -7,6 +7,7 @@ import sys from .jrnl import run from .args import parse_args +from .exception import JrnlError def configure_logger(debug=False): @@ -33,5 +34,9 @@ def cli(manual_args=None): return run(args) + except JrnlError as e: + print(e.message, file=sys.stderr) + return 1 + except KeyboardInterrupt: return 1 diff --git a/jrnl/config.py b/jrnl/config.py index da772927..9e57de3d 100644 --- a/jrnl/config.py +++ b/jrnl/config.py @@ -1,13 +1,77 @@ import logging +import os import sys import colorama import yaml +import xdg.BaseDirectory +from . import __version__ +from .exception import JrnlError from .color import ERROR_COLOR from .color import RESET_COLOR from .output import list_journals +# Constants +DEFAULT_CONFIG_NAME = "jrnl.yaml" +XDG_RESOURCE = "jrnl" + +DEFAULT_JOURNAL_NAME = "journal.txt" +DEFAULT_JOURNAL_KEY = "default" + + +def save_config(config): + config["version"] = __version__ + with open(get_config_path(), "w") as f: + yaml.safe_dump( + config, f, encoding="utf-8", allow_unicode=True, default_flow_style=False + ) + + +def get_config_path(): + try: + config_directory_path = xdg.BaseDirectory.save_config_path(XDG_RESOURCE) + except FileExistsError: + raise JrnlError( + "ConfigDirectoryIsFile", + config_directory_path=os.path.join( + xdg.BaseDirectory.xdg_config_home, XDG_RESOURCE + ), + ) + return os.path.join( + config_directory_path or os.path.expanduser("~"), DEFAULT_CONFIG_NAME + ) + + +def get_default_config(): + return { + "version": __version__, + "journals": {"default": get_default_journal_path()}, + "editor": os.getenv("VISUAL") or os.getenv("EDITOR") or "", + "encrypt": False, + "template": False, + "default_hour": 9, + "default_minute": 0, + "timeformat": "%Y-%m-%d %H:%M", + "tagsymbols": "@", + "highlight": True, + "linewrap": 79, + "indent_character": "|", + "colors": { + "date": "none", + "title": "none", + "body": "none", + "tags": "none", + }, + } + + +def get_default_journal_path(): + journal_data_path = xdg.BaseDirectory.save_data_path( + XDG_RESOURCE + ) or os.path.expanduser("~") + return os.path.join(journal_data_path, DEFAULT_JOURNAL_NAME) + def scope_config(config, journal_name): if journal_name not in config["journals"]: @@ -73,13 +137,11 @@ def update_config(config, new_config, scope, force_local=False): def get_journal_name(args, config): - from . import install - - args.journal_name = install.DEFAULT_JOURNAL_KEY + args.journal_name = DEFAULT_JOURNAL_KEY if args.text and args.text[0] in config["journals"]: args.journal_name = args.text[0] args.text = args.text[1:] - elif install.DEFAULT_JOURNAL_KEY not in config["journals"]: + elif DEFAULT_JOURNAL_KEY not in config["journals"]: print("No default journal configured.", file=sys.stderr) print(list_journals(config), file=sys.stderr) sys.exit(1) diff --git a/jrnl/exception.py b/jrnl/exception.py index f1a509f5..82a562a0 100644 --- a/jrnl/exception.py +++ b/jrnl/exception.py @@ -1,5 +1,6 @@ # Copyright (C) 2012-2021 jrnl contributors # License: https://www.gnu.org/licenses/gpl-3.0.html +import textwrap class UserAbort(Exception): @@ -10,3 +11,28 @@ class UpgradeValidationException(Exception): """Raised when the contents of an upgraded journal do not match the old journal""" pass + + +class JrnlError(Exception): + """Common exceptions raised by jrnl. """ + + def __init__(self, error_type, **kwargs): + self.error_type = error_type + self.message = self._get_error_message(**kwargs) + + def _get_error_message(self, **kwargs): + error_messages = { + "ConfigDirectoryIsFile": textwrap.dedent( + """ + The path to your jrnl configuration directory is a file, not a directory: + + {config_directory_path} + + Removing this file will allow jrnl to save its configuration. + """ + ) + } + + return error_messages[self.error_type].format(**kwargs) + + pass diff --git a/jrnl/install.py b/jrnl/install.py index a5023815..db4c0fba 100644 --- a/jrnl/install.py +++ b/jrnl/install.py @@ -8,86 +8,45 @@ import logging import os import sys -import xdg.BaseDirectory -import yaml - -from . import __version__ +from .config import DEFAULT_JOURNAL_KEY +from .config import get_config_path +from .config import get_default_config +from .config import get_default_journal_path from .config import load_config +from .config import save_config from .config import verify_config_colors from .exception import UserAbort from .prompt import yesno from .upgrade import is_old_version -DEFAULT_CONFIG_NAME = "jrnl.yaml" -DEFAULT_JOURNAL_NAME = "journal.txt" -DEFAULT_JOURNAL_KEY = "default" -XDG_RESOURCE = "jrnl" - -USER_HOME = os.path.expanduser("~") - -CONFIG_PATH = xdg.BaseDirectory.save_config_path(XDG_RESOURCE) or USER_HOME -CONFIG_FILE_PATH = os.path.join(CONFIG_PATH, DEFAULT_CONFIG_NAME) -CONFIG_FILE_PATH_FALLBACK = os.path.join(USER_HOME, ".jrnl_config") - -JOURNAL_PATH = xdg.BaseDirectory.save_data_path(XDG_RESOURCE) or USER_HOME -JOURNAL_FILE_PATH = os.path.join(JOURNAL_PATH, DEFAULT_JOURNAL_NAME) - - -default_config = { - "version": __version__, - "journals": {"default": JOURNAL_FILE_PATH}, - "editor": os.getenv("VISUAL") or os.getenv("EDITOR") or "", - "encrypt": False, - "template": False, - "default_hour": 9, - "default_minute": 0, - "timeformat": "%Y-%m-%d %H:%M", - "tagsymbols": "@", - "highlight": True, - "linewrap": 79, - "indent_character": "|", - "colors": { - "date": "none", - "title": "none", - "body": "none", - "tags": "none", - }, -} - def upgrade_config(config): """Checks if there are keys missing in a given config dict, and if so, updates the config file accordingly. This essentially automatically ports jrnl installations if new config parameters are introduced in later versions.""" + default_config = get_default_config() missing_keys = set(default_config).difference(config) if missing_keys: for key in missing_keys: config[key] = default_config[key] save_config(config) print( - f"[Configuration updated to newest version at {CONFIG_FILE_PATH}]", + f"[Configuration updated to newest version at {get_config_path()}]", file=sys.stderr, ) -def save_config(config): - config["version"] = __version__ - with open(CONFIG_FILE_PATH, "w") as f: - yaml.safe_dump( - config, f, encoding="utf-8", allow_unicode=True, default_flow_style=False - ) - - def load_or_install_jrnl(): """ If jrnl is already installed, loads and returns a config object. Else, perform various prompts to install jrnl. """ config_path = ( - CONFIG_FILE_PATH - if os.path.exists(CONFIG_FILE_PATH) - else CONFIG_FILE_PATH_FALLBACK + get_config_path() + if os.path.exists(get_config_path()) + else os.path.join(os.path.expanduser("~"), ".jrnl_config") ) + if os.path.exists(config_path): logging.debug("Reading configuration from file %s", config_path) config = load_config(config_path) @@ -128,8 +87,10 @@ def install(): _initialize_autocomplete() # Where to create the journal? - path_query = f"Path to your journal file (leave blank for {JOURNAL_FILE_PATH}): " - journal_path = os.path.abspath(input(path_query).strip() or JOURNAL_FILE_PATH) + default_journal_path = get_default_journal_path() + path_query = f"Path to your journal file (leave blank for {default_journal_path}): " + journal_path = os.path.abspath(input(path_query).strip() or default_journal_path) + default_config = get_default_config() default_config["journals"][DEFAULT_JOURNAL_KEY] = os.path.expanduser( os.path.expandvars(journal_path) ) diff --git a/jrnl/jrnl.py b/jrnl/jrnl.py index ad5b07d0..415200fa 100644 --- a/jrnl/jrnl.py +++ b/jrnl/jrnl.py @@ -11,6 +11,7 @@ from .color import ERROR_COLOR from .color import RESET_COLOR from .config import get_journal_name from .config import scope_config +from .config import get_config_path from .editor import get_text_from_editor from .editor import get_text_from_stdin from .exception import UserAbort @@ -228,7 +229,7 @@ def _edit_search_results(config, journal, old_entries, **kwargs): f""" [{ERROR_COLOR}ERROR{RESET_COLOR}: There is no editor configured.] - Please specify an editor in config file ({install.CONFIG_FILE_PATH}) + Please specify an editor in config file ({get_config_path()}) to use the --edit option. """, file=sys.stderr, diff --git a/jrnl/output.py b/jrnl/output.py index 43390346..60c5d5aa 100644 --- a/jrnl/output.py +++ b/jrnl/output.py @@ -23,13 +23,13 @@ def deprecated_cmd(old_cmd, new_cmd, callback=None, **kwargs): callback(**kwargs) -def list_journals(config): - from . import install +def list_journals(configuration): + from . import config """List the journals specified in the configuration file""" - result = f"Journals defined in {install.CONFIG_FILE_PATH}\n" - ml = min(max(len(k) for k in config["journals"]), 20) - for journal, cfg in config["journals"].items(): + result = f"Journals defined in {config.get_config_path()}\n" + ml = min(max(len(k) for k in configuration["journals"]), 20) + for journal, cfg in configuration["journals"].items(): result += " * {:{}} -> {}\n".format( journal, ml, cfg["journal"] if isinstance(cfg, dict) else cfg ) diff --git a/tests/test_exception.py b/tests/test_exception.py new file mode 100644 index 00000000..85eb77e9 --- /dev/null +++ b/tests/test_exception.py @@ -0,0 +1,19 @@ +import textwrap + +from jrnl.exception import JrnlError + + +def test_config_directory_exception_message(): + ex = JrnlError( + "ConfigDirectoryIsFile", config_directory_path="/config/directory/path" + ) + + assert ex.message == textwrap.dedent( + """ + The path to your jrnl configuration directory is a file, not a directory: + + /config/directory/path + + Removing this file will allow jrnl to save its configuration. + """ + )