Skip to content

Conversation

@ZeroX-DG
Copy link
Member

@ZeroX-DG ZeroX-DG commented Apr 20, 2018

This is a new feature and has been requested here:
#1830, #576

Basically this feature allow you to create snippet (like code snippet) and use it to boost up your typing speed.

Enough with words! here is the demo:
peek 2018-04-20 23-25

To use this feature, you will first need to type in the prefix of the snippet which is just a word to trigger the snippet content (in the example above it's ipsum) then press tab to expand the snippet

You can also define your own snippet at the snippet tab in setting

peek 2018-04-20 23-28

When you edit the snippet name or prefix or content, the snippet will automatically update so don't worry if you forget to hit the save button although there is no save button 😄

You can set more than just one prefix and separate them using commas like this:
peek 2018-05-02 10-25

You can delete the snippet by right click on the snippet and click delete

peek 2018-04-20 23-31

You can also create a note template using this feature. By adding the special string :{} you can set the cursor position in your template once it's generated.

peek 2018-05-17 00-17

The only problem is that I haven't add new details to the language file, will do it if I have time 😃

@Rokt33r Rokt33r self-requested a review April 21, 2018 08:12
@Rokt33r
Copy link
Member

Rokt33r commented Apr 21, 2018

@ZeroX-DG Wow, it looks nice!!

const { rulers, enableRulers } = this.props
const emptyChars = /\t|\s|\r|\n/
if (!fs.existsSync(consts.SNIPPET_FILE)) {
const defaultSnippets = [
Copy link
Contributor

Choose a reason for hiding this comment

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

That are quite a lot 'default' snippets not everybody needs. Nevertheless they will be inserted by default... not sure if that's a good idea

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right 😄 I think leaving just 1 default snippet is enough, will fix it soon

function getAppData () {
return process.env.APPDATA || (process.platform === 'darwin'
? process.env.HOME + 'Library/Preferences'
: require('os').homedir() + '/.config')
Copy link
Contributor

Choose a reason for hiding this comment

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

path.sep woulde be way better than a hardcoded '/' since that isn't platformindependent!

Copy link
Member Author

Choose a reason for hiding this comment

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

First time I heard of path.sep new stuff to learn! thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Using path.sep is not the best. Please use path.join() to concatenating path.

import crypto from 'crypto'
import consts from 'browser/lib/consts'

function createSnippet (snippets) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why didn't you write any tests?!
Tests are good!

Copy link
Member Author

Choose a reason for hiding this comment

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

oh I was busy at that time, I'll add some tests for it 😄

@kazup01 kazup01 added the awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. label Apr 22, 2018
@ZeroX-DG
Copy link
Member Author

@Rokt33r It's basically done, please tell me if you need anything

@Rokt33r Rokt33r added awaiting review ❇️ Pull request is awaiting a review. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Apr 28, 2018
@Rokt33r Rokt33r requested review from sosukesuzuki and removed request for Rokt33r April 28, 2018 14:55
Copy link
Member

@Rokt33r Rokt33r left a comment

Choose a reason for hiding this comment

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

I think we have to generate snippets.json file if it does not exist.

Error: ENOENT: no such file or directory, open '/Users/~/.config/Boostnote/snippets.json'

@Rokt33r Rokt33r added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels May 1, 2018
@ZeroX-DG
Copy link
Member Author

ZeroX-DG commented May 2, 2018

That's weird, I have make sure that the snippets.json will be created if it's not exist everytime Boostnote is executed. Look like the path in your error is wrong, I have adjusted it, maybe it's the cause to this problem

@ChasManRors
Copy link

I can't wait until this lands in the main distribution.

@ZeroX-DG
Copy link
Member Author

ZeroX-DG commented May 2, 2018

@ChasManRors Just a few more changes and it will be ready!
@Rokt33r I've disabled the code highlighting of codemirror in the SnippetEditor. It would be great if there is a language support feature for SnippetEditor but I'm having trouble with my school project, so right now, let just allow snippets to be in plan text only :'(

@IssueHuntBot IssueHuntBot mentioned this pull request May 16, 2018
Copy link
Member

@sosukesuzuki sosukesuzuki left a comment

Choose a reason for hiding this comment

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

Please confirm my reviews!

for (let i = 0; i < snippets.length; i++) {
if (snippets[i].prefix.indexOf(wordBeforeCursor.text) !== -1) {
if (snippets[i].content.indexOf(templateCursorString) !== -1) {
let snippetLines = snippets[i].content.split('\n')
Copy link
Member

Choose a reason for hiding this comment

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

please use const instead of let.

let cursorLineNumber = 0
let cursorLinePosition = 0
for (let j = 0; j < snippetLines.length; j++) {
let cursorIndex = snippetLines[j].indexOf(templateCursorString)
Copy link
Member

Choose a reason for hiding this comment

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

please use const instead of let.

const fs = require('sander')
const { remote } = require('electron')
const { app } = remote
const os = require('os')
Copy link
Member

Choose a reason for hiding this comment

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

Please remove os, because it is never used.

}).catch(err => { throw err })
}

renderSnippetList () {
Copy link
Member

Choose a reason for hiding this comment

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

I think that it is better than now, to extract renderSnippetList as stateless functional component.

renderSnippetList () {
const { snippets } = this.state
return (
<ul id='snippets' style={{height: 'calc(100% - 8px)', overflow: 'scroll', background: '#f5f5f5'}}>
Copy link
Member

Choose a reason for hiding this comment

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

If it is possible, I want you to write style to browser/main/modals/PreferencesModal/SnippetTab.styl.

<div styleName='SnippetEditor' ref='root' tabIndex='-1' style={{
fontFamily: fontFamily.join(', '),
fontSize: fontSize,
position: 'absolute',
Copy link
Member

Choose a reason for hiding this comment

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

please write style of position, width, height to other stylus file.

import React from 'react'
import CSSModules from 'browser/lib/CSSModules'
import styles from './SnippetTab.styl'
import fs from 'fs'
Copy link
Member

Choose a reason for hiding this comment

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

Please remove fs, because fs is never used.

import SnippetEditor from './SnippetEditor'
import i18n from 'browser/lib/i18n'
import dataApi from 'browser/main/lib/dataApi'
import consts from 'browser/lib/consts'
Copy link
Member

Choose a reason for hiding this comment

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

Please remove consts. because consts is never used.

import CodeMirror from 'codemirror'
import React from 'react'
import _ from 'lodash'
import fs from 'fs'
Copy link
Member

Choose a reason for hiding this comment

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

Please remove fs because fs is never used.

import React from 'react'
import _ from 'lodash'
import fs from 'fs'
import consts from 'browser/lib/consts'
Copy link
Member

Choose a reason for hiding this comment

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

Please remove consts because consts is never used.

import crypto from 'crypto'
import consts from 'browser/lib/consts'
import fs from 'fs'
const { ipcRenderer, remote } = require('electron')
Copy link
Member

Choose a reason for hiding this comment

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

remote is never used, please remove it.

@sosukesuzuki
Copy link
Member

Snippets tab you implemented is styled for theme of White and Default.
I want you to style for other themes.
2018-05-21 13 21 18

@ZeroX-DG
Copy link
Member Author

@sosukesuzuki I have changed my code, please take a look 😃

Copy link
Member

@sosukesuzuki sosukesuzuki left a comment

Choose a reason for hiding this comment

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

Thanks for your fix!
Please fix it a bit. Confirm my reviews:pray:
And, please confirm this comment.

import React from 'react'
import _ from 'lodash'
import styles from './SnippetTab.styl'
import CSSModules from 'react-css-modules'
Copy link
Member

Choose a reason for hiding this comment

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

please usebrowser/lib/CSSModules instead of react-css-modules.

this.handleSnippetNameOrPrefixChange()
}}
value={currentSnippet ? currentSnippet.name : ''}
onChange={e => { this.onSnippetNameOrPrefixChanged(e, 'name') }}
Copy link
Member

Choose a reason for hiding this comment

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

{ } is unnecessary. please write like below:

onChange={e => this.onSnippetNameOrPrefixChanged(e, 'name')}

this.handleSnippetNameOrPrefixChange()
}}
value={currentSnippet ? currentSnippet.prefix : ''}
onChange={e => { this.onSnippetNameOrPrefixChanged(e, 'prefix') }}
Copy link
Member

Choose a reason for hiding this comment

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

{ } is unnecessary. please write like below:

onChange={e => this.onSnippetNameOrPrefixChanged(e, 'prefix') }

@sosukesuzuki
Copy link
Member

@ZeroX-DG please fix a conflict!

@ZeroX-DG
Copy link
Member Author

@sosukesuzuki , I have confirmed you requests.

@sosukesuzuki
Copy link
Member

@ZeroX-DG
please apply style for monokai theme.
please use variables written in browser/styles/index.styl to apply styles for color themes.

@ZeroX-DG
Copy link
Member Author

@sosukesuzuki , Sorry for missing a theme, I've added the monokai theme & switched all colors to the color variables in the index.styl file

@Rokt33r Rokt33r merged commit e4d4041 into BoostIO:master Jun 1, 2018
@sosukesuzuki sosukesuzuki added next release (v0.11.6) and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Jun 14, 2018
@ZeroX-DG ZeroX-DG deleted the text-expansion-support branch July 26, 2019 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants