derekriemer / nvda-notepadPlusPlus

GNU General Public License v2.0
28 stars 20 forks source link

Web preview #11

Closed Robert-J-H closed 7 years ago

Robert-J-H commented 7 years ago

Hi Derek

Opens the content of the editor window in a browsable message (Single keypress) and in the default web browser (double keypress).

Possible enhancements:

The PR does not include translation related adaptions or updated help files. Regards Robert

derekriemer commented 7 years ago

@tuukkao any comments?

josephsl commented 7 years ago

Hi, authors should NEVER (ever) write localized readme.md in their code directly, as it’ll confuse the translations workflow. The ideal way is through the SVN. Thanks.

From: Derek Riemer [mailto:notifications@github.com] Sent: Friday, September 1, 2017 2:15 PM To: derekriemer/nvda-notepadPlusPlus nvda-notepadPlusPlus@noreply.github.com Cc: Joseph Lee joseph.lee22590@gmail.com; Mention mention@noreply.github.com Subject: Re: [derekriemer/nvda-notepadPlusPlus] Web preview (#11)

@derekriemer requested changes on this pull request.

Nice job. I added a few more notes. Ping me when you are ready for another review and I'll try to respond faster.


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136600743 :

@@ -148,31 +151,41 @@ def script_reportFindResult(self, gesture): speech.speakMessage(new.text) else:

Translators: Message shown when there are no more search results in this direction using the notepad++ find command.

  • speech.speakMessage(_("No more search results in this direction."))
  • speech.speakMessage(_("No more search results in this direction"))
  • Translators: when pressed, goes to the Next search result in Notepad++

Still waiting for you to remove the tab you inserted here in the middle of the line.


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136601240 :

  #Translators: when pressed, goes to      the Next search result in Notepad++
  • scriptreportFindResult.doc = ("Queries the next or previous search result and speaks the selection and current line of it.")
  • scriptreportFindResult.doc = ("Queries the next or previous search result and speaks the selection and current line of it") script_reportFindResult.category = "Notepad++"

make sure the crlf didn't change here.


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136602005 :

         repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)

I think this line has no affect, because the text info expands to story anyway here since you opted for POSITION_ALL, which means, give me a text range, expanded to all.


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136602192 :

         repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it back

What exactly do you mean by this? What about the sconsstruct?


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136602480 :

         repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it back

  • but we use regular expressions rather than .replace

  • raw = re.sub(r'[[!meta title=\"(.*)\"]]', r'# \1 #', raw)

I think I know what you are trying to do here now. Are you trying to replace a specific markdown tag with a heading 1 since the markdown processor in python doesn't have that?


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136602883 :

         repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it back

  • but we use regular expressions rather than .replace

  • raw = re.sub(r'[[!meta title=\"(.*)\"]]', r'# \1 #', raw)
  • Translators: The title of the browseable message

Please be slightly more specific, I.E.

Translators: The title of a browsable message to present a markdown preview.


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136603069 :

                 f= tempfile.NamedTemporaryFile(suffix='.html',delete=False)
  • f.write(html)
  • f.write(html.encode('utf-8'))

Nice catch.


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136603653 :

                 f.close()
  • we assume that the default aplication for *.html is a browser

  • The webrowser module does not always open the file with the standard browser

    os.startfile(f.name) remover=Timer(20.0, os.remove, [f.name])

I wish there was a more elegant way to do this, but after putting some thought into it, I don't know of a better way which wouldn't be a lot of extra work.


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136604248 :

                 f.close()
  • we assume that the default aplication for *.html is a browser

Nit: aplication -> application


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136604580 :

                 f.close()
  • we assume that the default aplication for *.html is a browser

  • The webrowser module does not always open the file with the standard browser

Thanks for clarifying this . Just for future reference, where did you find this?


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136604804 :

                 os.startfile(f.name)

remover=Timer(20.0, os.remove, [f.name]) remover.start() else:

  • ui.browseableMessage(html, "Preview of MarkDown Content as Html", True)
  • script_htmlPreview.doc = """Shows the Editor Window Content after converting to HTML.
  • Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser.
  • The temporary file is removed after 20 seconds"""
  • ui.browseableMessage(html, title, True)
  • Translators: when pressed once, opens a message box or, pressed twice, the default browser and interprets the Editor text as markdown/HTML

Nit: extra tab in the middle of translators comment.


In addon/appModules/notepad++/editWindow.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136664495 :

                 os.startfile(f.name)

remover=Timer(20.0, os.remove, [f.name]) remover.start() else:

  • ui.browseableMessage(html, "Preview of MarkDown Content as Html", True)
  • script_htmlPreview.doc = """Shows the Editor Window Content after converting to HTML.
  • Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser.
  • The temporary file is removed after 20 seconds"""
  • ui.browseableMessage(html, title, True)
  • Translators: when pressed once, opens a message box or, pressed twice, the default browser and interprets the Editor text as markdown/HTML

  • scripthtmlPreview.doc = ("""Shows the Editor Window Content after converting it to HTML. +Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser. +The temporary file is removed after 20 seconds.""")

Is this fact necessary to the average user?


In readme.md https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136664789 :

This add-on expects that Notepad++ is being used with the default shortcut keys. If this is not the case, please change this app module's key commands to reflect your Notepad++ commands as necessary in NVDA's input gestures dialog. -All of the add-ons commands are under the notepad++ section. +All of the add-ons commands are under the Notepad++ section.

What changed between these lines?


In readme.md https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136664974 :

+


  • Where it began...

  • A long time ago,

  • in a foreign country.

  • And where it went next

    1. First stage
    1. Second stage
  • Eventually it became

    • unordered
    • but still
    • a list
  • +

  • +# NonDdefault Notepad++ Keyboard Shortcuts

NIT: Wrong heading level, ddefault should be default.


In readme.md https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136665166 :

-This add-on improves the accessibility of notepad++. Notepad++ is a text editor for windows, and has many features. You can learn more about it at https://notepad-plus-plus.org/ +This add-on improves the accessibility of notepad++. Notepad++ is a text editor for windows, and has many features. You can learn more about it at https://notepad-plus-plus.org/

Hey, thanks.


In addon/doc/fr/readme.md https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136665322 :

@@ -0,0 +1,98 @@ +# Notepad++ Module complémentaire pour NVDA #

@josephsl https://github.com/josephsl is this how the system expects translations?


In buildVars.py https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136665482 :

  # version

I'll add you here once this merges, unless you want to now.


In readme.md https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136666021 :

Pressing nvda+shift+\ (back slash) at any time will report the following:

Nit: unbalanced parenthesis, although that was my bug I think.


In readme.md https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#discussion_r136666168 :

By Default, if you press control+f you bring up the find dialog. If you type text here and press enter, the text in the window is selected and the document is moved to the next search result. In Notepad++ you can press f3 or shift+f3 to repeat the search in the forward or backward direction respectively. NVDA will read both the current line, and the selection within the line which represents the found text.

-## Non-default Notepad++ keyboard shortcuts +### Preview of MarkDown or Hypertext as Webpage + +Notepad++ does natively not support MarkDown (*.md) with e.g. language highlighting.

Nit: natively not, -> not natively

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#pullrequestreview-60166885 , or mute the thread https://github.com/notifications/unsubscribe-auth/AHgLkOfuCauhqJsFsMmxUKTun5Eq6ZyNks5seHPFgaJpZM4O8SBl .

Robert-J-H commented 7 years ago

Hi Derek

Thanks for your review I will resolve it one by one. However, I think that I have to split up the preview commands into two separate scripts. It does not work as smoothly as the global plug-in.

So, I think it is better to add another shortcut, such as NVDA+H.

I've removed the tabs in the script docs where they span over more than one line, otherwise they will go into the translation. So, it is a compromise between code layout and proper input help string.

Also: shouldn't we replace Notepad++ with NotepadPlusPlus in order to make sure that it is always correctly read, no matter what the symbol level?

Have you finished the development of your features? I've seen that the keyboard mapper enhancement doesn't work when I switch everything to German. I've not tried the other three languages.

Other possible features:

Warning when the write mode switches from insert to overwrite (happens here all the time because insert is the shortcut for that and it is sometimes directly passed to Notepad++).

Cheers Robert

On 01/09/2017, Derek Riemer notifications@github.com wrote:

derekriemer requested changes on this pull request.

Nice job. I added a few more notes. Ping me when you are ready for another review and I'll try to respond faster.

@@ -148,31 +151,41 @@ def script_reportFindResult(self, gesture): speech.speakMessage(new.text) else:

Translators: Message shown when there are no more search results in

this direction using the notepad++ find command.

  • speech.speakMessage(_("No more search results in this direction."))
  • speech.speakMessage(_("No more search results in this direction"))
  • Translators: when pressed, goes to the Next search result in

    Notepad++

Still waiting for you to remove the tab you inserted here in the middle of the line.

 #Translators: when pressed, goes to    the Next search result in

Notepad++

  • scriptreportFindResult.doc = ("Queries the next or previous search result and speaks the selection and current line of it.")
  • scriptreportFindResult.doc = ("Queries the next or previous search result and speaks the selection and current line of it") script_reportFindResult.category = "Notepad++"

make sure the crlf didn't change here.

     repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)

I think this line has no affect, because the text info expands to story anyway here since you opted for POSITION_ALL, which means, give me a text range, expanded to all.

     repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it

    back

What exactly do you mean by this? What about the sconsstruct?

     repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it

    back

  • but we use regular expressions rather than .replace

  • raw = re.sub(r'[[!meta title=\"(.*)\"]]', r'# \1 #', raw)

I think I know what you are trying to do here now. Are you trying to replace a specific markdown tag with a heading 1 since the markdown processor in python doesn't have that?

     repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it

    back

  • but we use regular expressions rather than .replace

  • raw = re.sub(r'[[!meta title=\"(.*)\"]]', r'# \1 #', raw)
  • Translators: The title of the browseable message

Please be slightly more specific, I.E.

Translators: The title of a browsable message to present a markdown

preview.

         f= tempfile.NamedTemporaryFile(suffix='.html',delete=False)
  • f.write(html)
  • f.write(html.encode('utf-8'))

Nice catch.

         f.close()
  • we assume that the default aplication for *.html is a browser

  • The webrowser module does not always open the file with the standard

    browser os.startfile(f.name) remover=Timer(20.0, os.remove, [f.name])

I wish there was a more elegant way to do this, but after putting some thought into it, I don't know of a better way which wouldn't be a lot of extra work.

         f.close()
  • we assume that the default aplication for *.html is a browser

Nit: aplication -> application

         f.close()
  • we assume that the default aplication for *.html is a browser

  • The webrowser module does not always open the file with the standard

    browser

Thanks for clarifying this . Just for future reference, where did you find this?

         os.startfile(f.name)

remover=Timer(20.0, os.remove, [f.name]) remover.start() else:

  • ui.browseableMessage(html, "Preview of MarkDown Content as Html", True)
  • script_htmlPreview.doc = """Shows the Editor Window Content after converting to HTML.
  • Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser.
  • The temporary file is removed after 20 seconds"""
  • ui.browseableMessage(html, title, True)
  • Translators: when pressed once, opens a message box or, pressed twice,

    the default browser and interprets the Editor text as markdown/HTML

Nit: extra tab in the middle of translators comment.

         os.startfile(f.name)

remover=Timer(20.0, os.remove, [f.name]) remover.start() else:

  • ui.browseableMessage(html, "Preview of MarkDown Content as Html", True)
  • script_htmlPreview.doc = """Shows the Editor Window Content after converting to HTML.
  • Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser.
  • The temporary file is removed after 20 seconds"""
  • ui.browseableMessage(html, title, True)
  • Translators: when pressed once, opens a message box or, pressed twice,

    the default browser and interprets the Editor text as markdown/HTML

  • scripthtmlPreview.doc = ("""Shows the Editor Window Content after converting it to HTML. +Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser. +The temporary file is removed after 20 seconds.""")

Is this fact necessary to the average user?

This add-on expects that Notepad++ is being used with the default shortcut keys. If this is not the case, please change this app module's key commands to reflect your Notepad++ commands as necessary in NVDA's input gestures dialog. -All of the add-ons commands are under the notepad++ section. +All of the add-ons commands are under the Notepad++ section.

What changed between these lines?

+


  • Where it began...

  • A long time ago,

  • in a foreign country.

  • And where it went next

    1. First stage
    1. Second stage
  • Eventually it became

    • unordered
    • but still
    • a list
  • +

  • +# NonDdefault Notepad++ Keyboard Shortcuts

NIT: Wrong heading level, ddefault should be default.

-This add-on improves the accessibility of notepad++. Notepad++ is a text editor for windows, and has many features. You can learn more about it at https://notepad-plus-plus.org/ +This add-on improves the accessibility of notepad++. Notepad++ is a text editor for windows, and has many features. You can learn more about it at https://notepad-plus-plus.org/

Hey, thanks.

@@ -0,0 +1,98 @@ +# Notepad++ Module complémentaire pour NVDA #

@josephsl is this how the system expects translations?

 # version

I'll add you here once this merges, unless you want to now.

Pressing nvda+shift+\ (back slash) at any time will report the following:

  • the line number
  • the column number I.E. how far into the line you are. - the selection size, (number of characters horizontally selected, followed by the number of characters vertically selected, which would make a rectangle. This info is only reported if relevant. + the selection size, (number of characters horizontally selected, followed by the number of characters vertically selected, which would make a rectangle. This info is only reported if relevant.

Nit: unbalanced parenthesis, although that was my bug I think.

By Default, if you press control+f you bring up the find dialog. If you type text here and press enter, the text in the window is selected and the document is moved to the next search result. In Notepad++ you can press f3 or shift+f3 to repeat the search in the forward or backward direction respectively. NVDA will read both the current line, and the selection within the line which represents the found text.

-## Non-default Notepad++ keyboard shortcuts +### Preview of MarkDown or Hypertext as Webpage + +Notepad++ does natively not support MarkDown (*.md) with e.g. language highlighting.

Nit: natively not, -> not natively

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#pullrequestreview-60166885

Robert-J-H commented 7 years ago

I leave that to you. It is not such a huge contribution that it would merit explicit mentioning.

derekriemer commented 7 years ago

@josephsl commented on Sep 1, 2017, 3:36 PM MDT:

Hi, authors should NEVER (ever) write localized readme.md in their code directly, as it’ll confuse the translations workflow. The ideal way is through the SVN. Thanks.

Should I have him hold these translations until I hook this into the translation system then? After this merges, I'll be nuking keymapper, because I cannot get it accessible without it breaking later, then I'll request review, and it'll upload to the site, where I will then have it hooked in.

derekriemer commented 7 years ago

@Robert-J-H commented on Sep 1, 2017, 4:13 PM MDT:

Hi Derek

Thanks for your review I will resolve it one by one. However, I think that I have to split up the preview commands into two separate scripts. It does not work as smoothly as the global plug-in.

  • It is very hard to let the external browser be opened (especially for short text)
  • The internal browser is called anyway despite of my efforts to establish a delay (0.5 s by using threading.timer), it should actually be possible to cancel the countdown but it does, for some reason, not within the NVDA framework.

So, I think it is better to add another shortcut, such as NVDA+H.

I've removed the tabs in the script docs where they span over more than one line, otherwise they will go into the translation. So, it is a compromise between code layout and proper input help string.

Also: shouldn't we replace Notepad++ with NotepadPlusPlus in order to make sure that it is always correctly read, no matter what the symbol level?

Have you finished the development of your features? I've seen that the keyboard mapper enhancement doesn't work when I switch everything to German. I've not tried the other three languages.

Other possible features:

Warning when the write mode switches from insert to overwrite (happens here all the time because insert is the shortcut for that and it is sometimes directly passed to Notepad++).

  • When control-tabbing through the documents, the whole path is read and this makes it sometimes cumbersome to switch to a specific document. Therefore, you could alter it such that the filename is read first, followed by the full path.

Cheers Robert

On 01/09/2017, Derek Riemer notifications@github.com wrote:

derekriemer requested changes on this pull request.

Nice job. I added a few more notes. Ping me when you are ready for another review and I'll try to respond faster.

@@ -148,31 +151,41 @@ def script_reportFindResult(self, gesture): speech.speakMessage(new.text) else:

Translators: Message shown when there are no more search results in

this direction using the notepad++ find command.

  • speech.speakMessage(_("No more search results in this direction."))
  • speech.speakMessage(_("No more search results in this direction"))
  • Translators: when pressed, goes to the Next search result in

    Notepad++

Still waiting for you to remove the tab you inserted here in the middle of the line.

Translators: when pressed, goes to the Next search result in

Notepad++

  • scriptreportFindResult.doc = ("Queries the next or previous search result and speaks the selection and current line of it.")
  • scriptreportFindResult.doc = ("Queries the next or previous search result and speaks the selection and current line of it") script_reportFindResult.category = "Notepad++"

make sure the crlf didn't change here.

   repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)

I think this line has no affect, because the text info expands to story anyway here since you opted for POSITION_ALL, which means, give me a text range, expanded to all.

   repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it

    back

What exactly do you mean by this? What about the sconsstruct?

   repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it

    back

  • but we use regular expressions rather than .replace

  • raw = re.sub(r'[[!meta title=\"(.*)\"]]', r'# \1 #', raw)

I think I know what you are trying to do here now. Are you trying to replace a specific markdown tag with a heading 1 since the markdown processor in python doesn't have that?

   repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform it

    back

  • but we use regular expressions rather than .replace

  • raw = re.sub(r'[[!meta title=\"(.*)\"]]', r'# \1 #', raw)
  • Translators: The title of the browseable message

Please be slightly more specific, I.E.

Translators: The title of a browsable message to present a markdown

preview.

       f= tempfile.NamedTemporaryFile(suffix='.html',delete=False)
  • f.write(html)
  • f.write(html.encode('utf-8'))

Nice catch.

       f.close()
  • we assume that the default aplication for *.html is a browser

  • The webrowser module does not always open the file with the standard

    browser os.startfile(f.name) remover=Timer(20.0, os.remove, [f.name])

I wish there was a more elegant way to do this, but after putting some thought into it, I don't know of a better way which wouldn't be a lot of extra work.

       f.close()
  • we assume that the default aplication for *.html is a browser

Nit: aplication -> application

       f.close()
  • we assume that the default aplication for *.html is a browser

  • The webrowser module does not always open the file with the standard

    browser

Thanks for clarifying this . Just for future reference, where did you find this?

       os.startfile(f.name)

remover=Timer(20.0, os.remove, [f.name]) remover.start() else:

  • ui.browseableMessage(html, "Preview of MarkDown Content as Html", True)
  • script_htmlPreview.doc = """Shows the Editor Window Content after converting to HTML.
  • Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser.
  • The temporary file is removed after 20 seconds"""
  • ui.browseableMessage(html, title, True)
  • Translators: when pressed once, opens a message box or, pressed twice,

    the default browser and interprets the Editor text as markdown/HTML

Nit: extra tab in the middle of translators comment.

       os.startfile(f.name)

remover=Timer(20.0, os.remove, [f.name]) remover.start() else:

  • ui.browseableMessage(html, "Preview of MarkDown Content as Html", True)
  • script_htmlPreview.doc = """Shows the Editor Window Content after converting to HTML.
  • Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser.
  • The temporary file is removed after 20 seconds"""
  • ui.browseableMessage(html, title, True)
  • Translators: when pressed once, opens a message box or, pressed twice,

    the default browser and interprets the Editor text as markdown/HTML

  • scripthtmlPreview.doc = ("""Shows the Editor Window Content after converting it to HTML. +Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser. +The temporary file is removed after 20 seconds.""")

Is this fact necessary to the average user?

This add-on expects that Notepad++ is being used with the default shortcut keys. If this is not the case, please change this app module's key commands to reflect your Notepad++ commands as necessary in NVDA's input gestures dialog. -All of the add-ons commands are under the notepad++ section. +All of the add-ons commands are under the Notepad++ section.

What changed between these lines?

+


  • Where it began...

  • A long time ago,

  • in a foreign country.

  • And where it went next

    1. First stage
    1. Second stage
  • Eventually it became

    • unordered
    • but still
    • a list
  • +

  • +# NonDdefault Notepad++ Keyboard Shortcuts

NIT: Wrong heading level, ddefault should be default.

-This add-on improves the accessibility of notepad++. Notepad++ is a text editor for windows, and has many features. You can learn more about it at https://notepad-plus-plus.org/ +This add-on improves the accessibility of notepad++. Notepad++ is a text editor for windows, and has many features. You can learn more about it at https://notepad-plus-plus.org/

Hey, thanks.

@@ -0,0 +1,98 @@ +# Notepad++ Module complémentaire pour NVDA #

@josephsl is this how the system expects translations?

version

I'll add you here once this merges, unless you want to now.

Pressing nvda+shift+\ (back slash) at any time will report the following:

  • the line number
  • the column number I.E. how far into the line you are. - the selection size, (number of characters horizontally selected, followed by the number of characters vertically selected, which would make a rectangle. This info is only reported if relevant. + the selection size, (number of characters horizontally selected, followed by the number of characters vertically selected, which would make a rectangle. This info is only reported if relevant.

Nit: unbalanced parenthesis, although that was my bug I think.

By Default, if you press control+f you bring up the find dialog. If you type text here and press enter, the text in the window is selected and the document is moved to the next search result. In Notepad++ you can press f3 or shift+f3 to repeat the search in the forward or backward direction respectively. NVDA will read both the current line, and the selection within the line which represents the found text.

-## Non-default Notepad++ keyboard shortcuts +### Preview of MarkDown or Hypertext as Webpage + +Notepad++ does natively not support MarkDown (*.md) with e.g. language highlighting.

Nit: natively not, -> not natively

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#pullrequestreview-60166885

Why do you need to split into two scripts? is it double firing? That's a bit messy, but I can see what you mean. Do you need a few days for this?

Robert-J-H commented 7 years ago

It should all be fine now.

I hope that I caught all the corrections by now. The split is done as well.

(BTW: The NVDA+Shift+Backslash shortcut is not very good for alien keyboards, I think. I would have to press "NVDA+Control+Alt+LessThan" on my Swiss keyboard).

Have a nice evening Robert

On 05/09/2017, Derek Riemer notifications@github.com wrote:

@Robert-J-H commented on [Sep 1, 2017, 4:13 PM MDT](https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#issuecomment-326694685 "2017-09-01T22:13:14Z - Replied by Github Reply Comments"):

Hi Derek

Thanks for your review I will resolve it one by one. However, I think that I have to split up the preview commands into two separate scripts. It does not work as smoothly as the global plug-in.

  • It is very hard to let the external browser be opened (especially for short text)
  • The internal browser is called anyway despite of my efforts to establish a delay (0.5 s by using threading.timer), it should actually be possible to cancel the countdown but it does, for some reason, not within the NVDA framework.

So, I think it is better to add another shortcut, such as NVDA+H.

I've removed the tabs in the script docs where they span over more than one line, otherwise they will go into the translation. So, it is a compromise between code layout and proper input help string.

Also: shouldn't we replace Notepad++ with NotepadPlusPlus in order to make sure that it is always correctly read, no matter what the symbol level?

Have you finished the development of your features? I've seen that the keyboard mapper enhancement doesn't work when I switch everything to German. I've not tried the other three languages.

Other possible features:

Warning when the write mode switches from insert to overwrite (happens here all the time because insert is the shortcut for that and it is sometimes directly passed to Notepad++).

  • When control-tabbing through the documents, the whole path is read and this makes it sometimes cumbersome to switch to a specific document. Therefore, you could alter it such that the filename is read first, followed by the full path.

Cheers Robert

On 01/09/2017, Derek Riemer notifications@github.com wrote:

derekriemer requested changes on this pull request.

Nice job. I added a few more notes. Ping me when you are ready for another review and I'll try to respond faster.

@@ -148,31 +151,41 @@ def script_reportFindResult(self, gesture): speech.speakMessage(new.text) else:

Translators: Message shown when there are no more search results

in this direction using the notepad++ find command.

  • speech.speakMessage(_("No more search results in this direction."))
  • speech.speakMessage(_("No more search results in this direction"))
  • Translators: when pressed, goes to the Next search result in

    Notepad++

Still waiting for you to remove the tab you inserted here in the middle of the line.

Translators: when pressed, goes to the Next search result in

Notepad++

  • scriptreportFindResult.doc = ("Queries the next or previous search result and speaks the selection and current line of it.")
  • scriptreportFindResult.doc = ("Queries the next or previous search result and speaks the selection and current line of it") script_reportFindResult.category = "Notepad++"

make sure the crlf didn't change here.

  repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)

I think this line has no affect, because the text info expands to story anyway here since you opted for POSITION_ALL, which means, give me a text range, expanded to all.

  repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform

    it back

What exactly do you mean by this? What about the sconsstruct?

  repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform

    it back

  • but we use regular expressions rather than .replace

  • raw = re.sub(r'[[!meta title=\"(.*)\"]]', r'# \1 #', raw)

I think I know what you are trying to do here now. Are you trying to replace a specific markdown tag with a heading 1 since the markdown processor in python doesn't have that?

  repeatCount=scriptHandler.getLastScriptRepeatCount()
  • raw=self.windowText
  • ti = self.makeTextInfo(textInfos.POSITION_ALL)
  • ti.expand(textInfos.UNIT_STORY)
  • raw = ti.text
  • the replacement in sconstruct can't be rendered, so we transform

    it back

  • but we use regular expressions rather than .replace

  • raw = re.sub(r'[[!meta title=\"(.*)\"]]', r'# \1 #', raw)
  • Translators: The title of the browseable message

Please be slightly more specific, I.E.

Translators: The title of a browsable message to present a markdown

preview.

      f= tempfile.NamedTemporaryFile(suffix='.html',delete=False)
  • f.write(html)
  • f.write(html.encode('utf-8'))

Nice catch.

      f.close()
  • we assume that the default aplication for *.html is a browser

  • The webrowser module does not always open the file with the

    standard browser os.startfile(f.name) remover=Timer(20.0, os.remove, [f.name])

I wish there was a more elegant way to do this, but after putting some thought into it, I don't know of a better way which wouldn't be a lot of extra work.

      f.close()
  • we assume that the default aplication for *.html is a browser

Nit: aplication -> application

      f.close()
  • we assume that the default aplication for *.html is a browser

  • The webrowser module does not always open the file with the

    standard browser

Thanks for clarifying this . Just for future reference, where did you find this?

      os.startfile(f.name)

remover=Timer(20.0, os.remove, [f.name]) remover.start() else:

  • ui.browseableMessage(html, "Preview of MarkDown Content as Html", True)
  • script_htmlPreview.doc = """Shows the Editor Window Content after converting to HTML.
  • Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser.
  • The temporary file is removed after 20 seconds"""
  • ui.browseableMessage(html, title, True)
  • Translators: when pressed once, opens a message box or, pressed

    twice, the default browser and interprets the Editor text as markdown/HTML

Nit: extra tab in the middle of translators comment.

      os.startfile(f.name)

remover=Timer(20.0, os.remove, [f.name]) remover.start() else:

  • ui.browseableMessage(html, "Preview of MarkDown Content as Html", True)
  • script_htmlPreview.doc = """Shows the Editor Window Content after converting to HTML.
  • Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser.
  • The temporary file is removed after 20 seconds"""
  • ui.browseableMessage(html, title, True)
  • Translators: when pressed once, opens a message box or, pressed

    twice, the default browser and interprets the Editor text as markdown/HTML

  • scripthtmlPreview.doc = ("""Shows the Editor Window Content after converting it to HTML. +Pressing once shows it within the internal Browser, Pressing twice sends it to the default Browser. +The temporary file is removed after 20 seconds.""")

Is this fact necessary to the average user?

This add-on expects that Notepad++ is being used with the default shortcut keys. If this is not the case, please change this app module's key commands to reflect your Notepad++ commands as necessary in NVDA's input gestures dialog. -All of the add-ons commands are under the notepad++ section. +All of the add-ons commands are under the Notepad++ section.

What changed between these lines?

+


  • Where it began...

  • A long time ago,

  • in a foreign country.

  • And where it went next

    1. First stage
    1. Second stage
  • Eventually it became

    • unordered
    • but still
    • a list
  • +

  • +# NonDdefault Notepad++ Keyboard Shortcuts

NIT: Wrong heading level, ddefault should be default.

-This add-on improves the accessibility of notepad++. Notepad++ is a text editor for windows, and has many features. You can learn more about it at https://notepad-plus-plus.org/ +This add-on improves the accessibility of notepad++. Notepad++ is a text editor for windows, and has many features. You can learn more about it at https://notepad-plus-plus.org/

Hey, thanks.

@@ -0,0 +1,98 @@ +# Notepad++ Module complémentaire pour NVDA #

@josephsl is this how the system expects translations?

version

I'll add you here once this merges, unless you want to now.

Pressing nvda+shift+\ (back slash) at any time will report the following:

  • the line number
  • the column number I.E. how far into the line you are. - the selection size, (number of characters horizontally selected, followed by the number of characters vertically selected, which would make a rectangle. This info is only reported if relevant. + the selection size, (number of characters horizontally selected, followed by the number of characters vertically selected, which would make a rectangle. This info is only reported if relevant.

Nit: unbalanced parenthesis, although that was my bug I think.

By Default, if you press control+f you bring up the find dialog. If you type text here and press enter, the text in the window is selected and the document is moved to the next search result. In Notepad++ you can press f3 or shift+f3 to repeat the search in the forward or backward direction respectively. NVDA will read both the current line, and the selection within the line which represents the found text.

-## Non-default Notepad++ keyboard shortcuts +### Preview of MarkDown or Hypertext as Webpage + +Notepad++ does natively not support MarkDown (*.md) with e.g. language highlighting.

Nit: natively not, -> not natively

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#pullrequestreview-60166885

Why do you need to split into two scripts? is it double firing? That's a bit messy, but I can see what you mean. Do you need a few days for this?

-- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#issuecomment-327243082

derekriemer commented 7 years ago

Oh dear, thanks for letting me know. What key would be better? I'm going to yank this out, then test it then, I'll merge it if the appVeyor builds stop failing. I'm not sure why they are, but your last two commits caused build failures on my CI, so I need to go look at what caused them. I don't think it's your fault.

derekriemer commented 7 years ago

never mind, last build worked. all good.

derekriemer commented 7 years ago

The build is in the queue

derekriemer commented 7 years ago

https://ci.appveyor.com/project/derekriemer/nvda-notepadplusplus/build/artifacts

Robert-J-H commented 7 years ago

Excellente!

I think the build error was because of a wrong character that I've accidently inserted. I converted all to ANSI after I'd seen it and it seemed to do the trick.

Thanks, now we have a base for further enhancements.

Robert

On 05/09/2017, Derek Riemer notifications@github.com wrote:

https://ci.appveyor.com/project/derekriemer/nvda-notepadplusplus/build/artifacts

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/derekriemer/nvda-notepadPlusPlus/pull/11#issuecomment-327257083

derekriemer commented 5 years ago

This is being removed when I upgrade this addon to work in python3, because the authors have decided that it is out of scope for this addon, and will be a burden to maintain. Please create your own globalPlugin with this support.