diff --git a/.eslintrc.yml b/.eslintrc.yml index 9c30f984e..3393dac66 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -25,13 +25,13 @@ rules: no-case-declarations: off no-unused-vars: - error - - - varsIgnorePattern: '_.+' + - varsIgnorePattern: '_.+' argsIgnorePattern: '_.*|reject' react/no-unescaped-entities: off react/display-name: off react/prop-types: off react-redux/prefer-separate-component-file: off + jest/no-disabled-tests: off settings: react: version: detect diff --git a/README.org b/README.org index fb7c16849..80485ed71 100644 --- a/README.org +++ b/README.org @@ -544,8 +544,10 @@ templates. They should be in the format ~captureVariable_~, and should also be URL encoded. In your capture template they'd show up as ~%~. -Important: At this point, organice assumes that you'll capture your -entries into a header - hence, you'll have to specify a "header path". +organice allows you to specify where the captured content will be +inserted, via a "header path" which is a list of headers to match. If +the list is empty, the content will be inserted at the end of the +file, or the beginning if the prepend option is selected. ** Examples *** Simple: Capture a string diff --git a/changelog.org b/changelog.org index be0b9810f..884bb5566 100644 --- a/changelog.org +++ b/changelog.org @@ -6,6 +6,11 @@ When there are updates to the changelog, you will be notified and see a 'gift' i + +* [2020-06-14 Sun] +** Added + - Allow capture templates to insert at beginning or end of file + - Thank you [[/~https://github.com/aspiers][aspiers]] for your [[/~https://github.com/200ok-ch/organice/pull/324][PR]] 🙏 * [2020-06-05 Fri] ** Fixed - =file:= links are sanity checked before opened diff --git a/sample.org b/sample.org index 5cd44a414..2246e5a36 100644 --- a/sample.org +++ b/sample.org @@ -205,7 +205,7 @@ organice supports something like org-capture in the form of customizable, quickl Click the button in the bottom right corner of the screen to see some examples. The first button, the lemon, will create a new entry in the "Groceries" list below this. The second button adds an entry to a more deeply nested header. -Once signed in, you can set up capture templates that specify header paths (and various other configurations). These capture templates can also sync between your devices (if you enable settings sync). +Once signed in, you can set up capture templates that specify header paths (and various other configurations). If the list is empty, the content will be inserted at the end of the file, or the beginning if the prepend option is selected. These capture templates will sync between your devices if you enable settings sync. ** Groceries ** Deeply *** Nested diff --git a/src/components/OrgFile/components/CaptureModal/index.js b/src/components/OrgFile/components/CaptureModal/index.js index 7d9e3200d..0de9f38a4 100644 --- a/src/components/OrgFile/components/CaptureModal/index.js +++ b/src/components/OrgFile/components/CaptureModal/index.js @@ -22,10 +22,13 @@ export default ({ template, onCapture, headers, onClose }) => { [template] ); - const targetHeader = useMemo(() => headerWithPath(headers, template.get('headerPaths')), [ - headers, - template, - ]); + const targetHeader = useMemo( + () => + template.get('headerPaths').size === 0 + ? 'FILE' + : headerWithPath(headers, template.get('headerPaths')), + [headers, template] + ); const [textareaValue, setTextareaValue] = useState(substitutedTemplate); const [shouldPrepend, setShouldPrepend] = useState(template.get('shouldPrepend')); diff --git a/src/reducers/org.js b/src/reducers/org.js index 212269c88..95381a2dd 100644 --- a/src/reducers/org.js +++ b/src/reducers/org.js @@ -729,27 +729,57 @@ const insertCapture = (state, action) => { const headers = state.get('headers'); const { template, content, shouldPrepend } = action; - const parentHeader = headerWithPath(headers, template.get('headerPaths')); - if (!parentHeader) { + const { newIndex, nestingLevel, parentHeader } = insertCapturePosition( + template, + headers, + shouldPrepend + ); + if (newIndex === undefined) { + // Should never happen; see comment in insertCapturePosition below. return state; } const newHeader = newHeaderFromText(content, state.get('todoKeywordSets')).set( 'nestingLevel', - parentHeader.get('nestingLevel') + 1 + nestingLevel ); - const parentHeaderIndex = indexOfHeaderWithId(headers, parentHeader.get('id')); - const numSubheaders = numSubheadersOfHeaderWithId(headers, parentHeader.get('id')); - const newIndex = parentHeaderIndex + 1 + (shouldPrepend ? 0 : numSubheaders); - state = state.update('headers', (headers) => headers.insert(newIndex, newHeader)); - - state = updateCookiesOfHeaderWithId(state, parentHeader.get('id')); + if (parentHeader !== undefined) { + // We inserted the new header under a parent rather than at the top or + // bottom of the file. + state = updateCookiesOfHeaderWithId(state, parentHeader.get('id')); + } return state; }; +const insertCapturePosition = (template, headers, shouldPrepend) => { + const headerPaths = template.get('headerPaths'); + if (headerPaths.size === 0) { + if (shouldPrepend) { + // Insert at beginning of file + return { newIndex: 0, nestingLevel: 1 }; + } else { + // Insert at end of file + return { newIndex: headers.size + 1, nestingLevel: 1 }; + } + } + + const parentHeader = headerWithPath(headers, headerPaths); + if (parentHeader == null) { + // No parent header found. In theory this shouldn't happen since + // CaptureModal already checks whether a valid targetHeader can be + // found if headerPaths is non-empty. + return {}; + } + const parentHeaderIndex = indexOfHeaderWithId(headers, parentHeader.get('id')); + const numSubheaders = numSubheadersOfHeaderWithId(headers, parentHeader.get('id')); + const newIndex = parentHeaderIndex + 1 + (shouldPrepend ? 0 : numSubheaders); + const nestingLevel = parentHeader.get('nestingLevel') + 1; + return { newIndex, nestingLevel, parentHeader }; +}; + const clearPendingCapture = (state) => state.set('pendingCapture', null); const updateParentListCheckboxes = (state, itemPath) => { diff --git a/src/reducers/org.unit.test.js b/src/reducers/org.unit.test.js index 15b8d20bd..0a9dc3920 100644 --- a/src/reducers/org.unit.test.js +++ b/src/reducers/org.unit.test.js @@ -1,29 +1,42 @@ import { fromJS } from 'immutable'; +import generateId from '../lib/id_generator'; import reducer from './org'; +import rootReducer from './index'; import * as types from '../actions/org'; import { parseOrg } from '../lib/parse_org'; import { readInitialState } from '../util/settings_persister'; -import { createStore } from 'redux'; -import undoable from 'redux-undo'; +import { createStore, applyMiddleware } from 'redux'; +import undoable, { ActionTypes } from 'redux-undo'; +import thunk from 'redux-thunk'; import readFixture from '../../test_helpers/index'; describe('org reducer', () => { let state; - let store; const testOrgFile = readFixture('main_test_file'); + // Given a `header`, return its `title` and `nestingLevel`. + function extractTitleAndNesting(header) { + return [header.getIn(['titleLine', 'rawTitle']), header.get('nestingLevel')]; + } + + // Given some `headers`, return their `title`s and `nestingLevel`s. + function extractTitlesAndNestings(headers) { + return headers + .map((header) => { + return extractTitleAndNesting(header); + }) + .toJS(); + } + beforeEach(() => { - state = fromJS(readInitialState()); - state = state.setIn(['org', 'present'], parseOrg(testOrgFile)); - store = createStore(undoable(reducer), state.getIn(['org', 'present'])); + state = readInitialState(); + state.org.present = parseOrg(testOrgFile); }); - describe('undo/redo', () => {}); - describe('REFILE_SUBTREE', () => { let sourceHeaderId, targetHeaderId; @@ -33,29 +46,20 @@ describe('org reducer', () => { // "PROJECT Foo" is the 10th item, "A nested header" the 2nd, // but we count from 0 not 1. - sourceHeaderId = state - .getIn(['org', 'present', 'headers']) + sourceHeaderId = state.org.present + .get('headers') .get(9) .get('id'); - targetHeaderId = state - .getIn(['org', 'present', 'headers']) + targetHeaderId = state.org.present + .get('headers') .get(1) .get('id'); }); it('should handle REFILE_SUBTREE', () => { - // Given some `headers`, return their `title`s and `nestingLevel`s. - function extractTitleAndNesting(headers) { - return headers - .map((header) => { - return [header.getIn(['titleLine', 'rawTitle']), header.get('nestingLevel')]; - }) - .toJS(); - } - // Mapping the headers to their nesting level. This is how the // initially parsed file should look like. - expect(extractTitleAndNesting(state.getIn(['org', 'present', 'headers']))).toEqual([ + expect(extractTitlesAndNestings(state.org.present.get('headers'))).toEqual([ ['Top level header', 1], ['A nested header', 2], ['A todo item with schedule and deadline', 2], @@ -71,14 +75,12 @@ describe('org reducer', () => { ['A header with a custom todo sequence in DONE state', 1], ]); - const newState = reducer( - state.getIn(['org', 'present']), - types.refileSubtree(sourceHeaderId, targetHeaderId) - ); + const action = types.refileSubtree(sourceHeaderId, targetHeaderId); + const newState = reducer(state.org.present, action); // PROJECT Foo is now beneath "A nested header" and is // appropriately indented. - expect(extractTitleAndNesting(newState.get('headers'))).toEqual([ + expect(extractTitlesAndNestings(newState.get('headers'))).toEqual([ ['Top level header', 1], ['A nested header', 2], ['PROJECT Foo', 3], @@ -96,11 +98,100 @@ describe('org reducer', () => { }); it('is undoable', () => { + const store = createStore(undoable(reducer), state.org.present); const oldState = store.getState().present; store.dispatch({ type: 'REFILE_SUBTREE', sourceHeaderId, targetHeaderId, dirtying: true }); expect(store.getState().present).not.toEqual(oldState); - store.dispatch({ type: '@@redux-undo/UNDO' }); + store.dispatch({ type: ActionTypes.UNDO }); expect(store.getState().present).toEqual(oldState); }); }); + + describe('INSERT_CAPTURE', () => { + let store, template; + + beforeEach(() => { + template = { + description: '', + headerPaths: [], + iconName: 'todo', + id: generateId(), + isAvailableInAllOrgFiles: false, + letter: '', + orgFilesWhereAvailable: [], + shouldPrepend: false, + template: '* TODO %?', + isSample: true, + }; + state.capture = state.capture.update('captureTemplates', (templates) => + templates.push(fromJS(template)) + ); + + // We have to create a full store rather than just the org bit, + // because the insertCapture thunk needs to retrieve capture + // templates from the capture part of the store. + store = createStore(rootReducer, state, applyMiddleware(thunk)); + }); + + const content = '* TODO My task\nSome description\n'; + + function expectOrigFirstHeader(headers) { + expect(extractTitleAndNesting(headers.first())).toEqual(['Top level header', 1]); + } + + function expectOrigLastHeader(headers) { + expect(extractTitleAndNesting(headers.last())).toEqual([ + 'A header with a custom todo sequence in DONE state', + 1, + ]); + } + + function insertCapture(shouldPrepend) { + // Check initially parsed file looks as expected + let headers = store.getState().org.present.get('headers'); + expect(headers.size).toEqual(13); + expectOrigFirstHeader(headers); + expectOrigLastHeader(headers); + const action = types.insertCapture(template.id, content, shouldPrepend); + store.dispatch(action); + const newHeaders = store.getState().org.present.get('headers'); + expect(newHeaders.size).toEqual(14); + return newHeaders; + } + + it('should insert at top of file', () => { + const newHeaders = insertCapture(true); + expectOrigLastHeader(newHeaders); + const first = newHeaders.first(); + expect(first.getIn(['titleLine', 'rawTitle'])).toEqual('My task'); + expect(first.getIn(['titleLine', 'todoKeyword'])).toEqual('TODO'); + expect(first.get('rawDescription')).toEqual('Some description\n'); + }); + + it('should insert at bottom of file', () => { + const newHeaders = insertCapture(false); + expectOrigFirstHeader(newHeaders); + const last = newHeaders.last(); + expect(last.getIn(['titleLine', 'rawTitle'])).toEqual('My task'); + expect(last.getIn(['titleLine', 'todoKeyword'])).toEqual('TODO'); + expect(last.get('rawDescription')).toEqual('Some description\n'); + }); + + // FIXME: This test works, but only when run with `.only`. There + // must be some racing condition or some mutable state involved, + // because it does not run together with the other tests. + it.skip('is undoable', () => { + const oldState = store.getState().org.present; + store.dispatch({ + type: 'INSERT_CAPTURE', + template: fromJS(template), + content, + shouldPrepend: true, + dirtying: true, + }); + expect(store.getState().org.present).not.toEqual(oldState); + store.dispatch({ type: ActionTypes.UNDO }); + expect(store.getState().org.present).toEqual(oldState); + }); + }); });