Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

svn urls not links since 7.9.1 #9155

Closed
nikita-leontiev opened this issue Nov 14, 2020 · 21 comments
Closed

svn urls not links since 7.9.1 #9155

nikita-leontiev opened this issue Nov 14, 2020 · 21 comments
Labels
URL parser Issue related to the new URL parser

Comments

@nikita-leontiev
Copy link

nikita-leontiev commented Nov 14, 2020

Description of the Issue

Since 7.9.1 svn urls are not links any more, so can't be open, for example, via TortoiseSVN.
Suppose related to #9090

Steps to Reproduce the Issue

Add any svn URL to editor, for example: svn://svn.notepad-plus-plus.org

Expected Behavior

Link.

Actual Behavior

Simple text.

Debug Information

Notepad++ v7.9.1 (32-bit)
Build time : Nov 2 2020 - 01:03:56
Path : C:\Program Files (x86)\Notepad++\notepad++.exe
Admin mode : ON
Local Conf mode : OFF
OS Name : Windows 7 Home Premium (64-bit)
OS Build : 7601.0
Current ANSI codepage : 1251
Plugins : DSpellCheck.dll mimeTools.dll NppConverter.dll NppExport.dll NPPJSONViewer.dll

@donho
Copy link
Member

donho commented Nov 14, 2020

In order to fix several bugs of URL link, the parsers of URL has been rewritten (by @Uhf7).
The current implementation handles much better URL. OTOH, it has some limit to support other protocol, like svn://.
@Uhf7 please correct/complete if what I mentioned above is not accurate.

@Uhf7
Copy link
Contributor

Uhf7 commented Nov 15, 2020

That's correct so far. The previous regex based parser accepted nearly everything as scheme name, the new version limits the allowed scheme names to the schemes which can be handled by the installed web browser and the installed email software.

svn: is a custom URL scheme, which may be installed on computers, but it is not available "out of the box". That may be one reason, why custom URL schemes are not detected by MS VC++, MS Outlook or MS Word. The other reason is, that every URL scheme actually has it's own format, although the svn: scheme format looks very similar to the http: scheme format.

How to get custom schemes supported? I can imagine a configurable list of custom schemes, which would then be handled like the http: scheme, if this is desperately needed. I cannot imagine to parse each and every custom scheme installed on a given computer or even check for the custom schemes installed. Many schemes I see on my computer seem to be for internal use by the software which installed them.

@FreeAndNil
Copy link

@Uhf7 How about a checkbox for enabling the old behavior?

@donho
Copy link
Member

donho commented Nov 17, 2020

@Uhf7

I can imagine a configurable list of custom schemes, which would then be handled like the http: scheme, if this is desperately needed. I cannot imagine to parse each and every custom scheme installed on a given computer or even check for the custom schemes installed.

Sounds reasonable.

@FreeAndNil

How about a checkbox for enabling the old behavior?

Unfortunately no. It's impossible to bring back the old, inaccurate detection.

@ThomasNygards
Copy link

Similar issue for me. Literally added a custom protocol yesterday using this:
https://stackoverflow.com/questions/389204/how-do-i-create-my-own-url-protocol-e-g-so

@andry81
Copy link

andry81 commented Nov 19, 2020

Unfortunately no. It's impossible to bring back the old, inaccurate detection.

For the sake of another inaccurate detection?

@donho
Copy link
Member

donho commented Nov 21, 2020

@andry81 @FreeAndNil
Sorry I wasn't clear. I meant there's no way to add an option for this issue.
OTOH, we can add the protocols we need with the old detection way upon the new detection (if it's refused by the new detection).
What do you think @Uhf7 ?

@andry81
Copy link

andry81 commented Nov 21, 2020

Sorry I wasn't clear. I meant there's no way to add an option for this issue.

What about to delegate all not known extensions to a browser or Windows Explorer?

@Uhf7
Copy link
Contributor

Uhf7 commented Nov 21, 2020

What about to delegate all not known extensions to a browser or Windows Explorer?

Notepad++ calls only the Windows shell, which decides, what software is to start.

After using a computer some time and installing some software, there are surely hundreds of custom schemes registered. Notepad++ cannot check them all, we don't want await a registry scan at each start.

There is a second problem: It may be the case, that software is installed, which is actually not much valued by the user of the computer. We all know, how eagerly some service providers install their preferred view of the world. If we would make all installed schemes clickable, then we may have links to something, which we do not want to run under any circumstances.

What do you think @Uhf7 ?

I don't think we need two different parsers to solve this. I just tested a suitable solution for the problem. It is a configuration string, containing a space-separated list of additional schemes the user wants to allow. In the code it looks like this:

generic_string const mySchemes = L"svn:// xyz: abc://"

As you notice, this configuration contains the slashes after the scheme, so that there is a chance to use it for custom schemes without // too (A build-in scheme without // is mailto:).

This configuration would need a place somewhere. Two ways:

  • a hidden configuration, which can be modified by editing the config.xml.
  • an additional parameter in the MISC. page of settings. Here we have two sub-choices too:
    • below "workspace file ext." (not the best place logically, but low impact on the menu)
    • below "Enable fullbox mode" (increases the height of the Preferences menu).

@andry81
Copy link

andry81 commented Nov 21, 2020

Notepad++ cannot check them all, we don't want await a registry scan at each start.

Why not run scan in parallel thread and update text links in the text? Why not to put the result of a scan into shared memory between multiple Notepad++ instances? Or even run a mini service to hold intermediate parse results?

There is a second problem: It may be the case, that software is installed, which is actually not much valued by the user of the computer. We all know, how eagerly some service providers install their preferred view of the world. If we would make all installed schemes clickable, then we may have links to something, which we do not want to run under any circumstances.

If you go "out of NP" by open a link (ctrl+click or double-click), then you can do it like in the browsers: open popup/sub menu to select variants with descriptions what they does.

@donho
Copy link
Member

donho commented Nov 21, 2020

@Uhf7

I don't think we need two different parsers to solve this. I just tested a suitable solution for the problem. It is a configuration string, containing a space-separated list of additional schemes the user wants to allow. In the code it looks like this:
generic_string const mySchemes = L"svn:// xyz: abc://"
As you notice, this configuration contains the slashes after the scheme, so that there is a chance to use it for custom schemes without // too (A build-in scheme without // is mailto:).

Sounds great! But in this case, why don't we make some default schemes (like "svn://" & "mailto://") plus customizable schemes, which are added by users via MISC of preferences dialog?

This configuration would need a place somewhere. Two ways:

  • a hidden configuration, which can be modified by editing the config.xml.
  • an additional parameter in the MISC. page of settings. Here we have two sub-choices too:
  • below "workspace file ext." (not the best place logically, but low impact on the menu)
  • below "Enable fullbox mode" (increases the height of the Preferences menu).

below "Enable fullbox mode" it makes more sense to me.
I can take in charge this part after you adding <GUIConfig name="URL customized schemes"></GUIConfig> settings in config.xml

@Uhf7
Copy link
Contributor

Uhf7 commented Nov 21, 2020

Why not run scan in parallel thread ...

Because the likelihood of crashes increases with the complexity of the software. And, as I mentioned, I don't want to run most of the software, which installed schemes on my computer. Under no circumstances. So I would need a exclude list then.

popup/sub menu to select variants with descriptions ...

This would make the URLs more than one double-click away for the user, so some users will complain about the extra layer in the operation.

And, new point: We are not talking about only 100 schemes, it is more like about 500 to 2000 schemes. About 10 percent of them may work as clickable links, the rest of them was installed for internal use of the software which installed them. Too hot for me.

@Uhf7
Copy link
Contributor

Uhf7 commented Nov 21, 2020

@donho

I can take in charge this part after you adding <GUIConfig name="URL customized schemes"></GUIConfig> settings in config.xml.

Thank you. This makes it much easier to me. PR is coming soon.

@Uhf7
Copy link
Contributor

Uhf7 commented Nov 21, 2020

@donho

Why don't we make some default schemes (like "svn://" & "mailto://") plus customizable schemes, which are added by users via MISC of preferences dialog?

We can specify "svn:// pdf://" as default value for the setting, right? The user can then extent this. "mailto:" is already standard. Or do you want to make the standard schemes excludable too?

@donho
Copy link
Member

donho commented Nov 21, 2020

@Uhf7

We can specify "svn:// pdf://" as default value for the setting, right? The user can then extent this. "mailto:" is already standard.

👍 they will be served as example furthermore.

Or do you want to make the standard schemes excludable too?

Sorry, I don't follow you ?

@Uhf7
Copy link
Contributor

Uhf7 commented Nov 21, 2020

@donho

Make the standard schemes excludable would mean we accept only the schemes which are specified in the configuration. To keep the behavior of 7.9.1, the configuration would have be:
ftp:// http:// https:// mailto: file://.
The user would then be able to de-select the standard schemes too.

The drawback is, that nothing works, if this configuration is empty. And we could end up to have to gray out the "Enable clickable links" box in the Preferences menu, if this configuration is empty. So I would prefer just to extend the standard schemes by the configuration.

@donho
Copy link
Member

donho commented Nov 21, 2020

@Uhf7

The drawback is, that nothing works, if this configuration is empty. And we could end up to have to gray out the "Enable clickable links" box in the Preferences menu, if this configuration is empty. So I would prefer just to extend the standard schemes by the configuration.

That makes perfect sense. So let's do it in this way.

@donho
Copy link
Member

donho commented Nov 21, 2020

Related: #9172

@donho donho closed this as completed in 5168bdb Nov 22, 2020
donho added a commit to donho/notepad-plus-plus that referenced this issue Nov 25, 2020
@ArkadiuszMichalski
Copy link
Contributor

@donho It will not be possible #9155 (comment)? We can't turn off some of default ftp:// http:// https:// mailto: file://?
Frome last PR actual list is svn:// cvs:// git:// imap:// irc:// irc6:// ircs:// ldap:// ldaps:// news: telnet:// gopher:// ssh:// sftp:// smb:// skype: snmp:// spotify: steam:// sms: slack:// chrome:// bitcoin:.

@donho
Copy link
Member

donho commented Nov 29, 2020

@ArkadiuszMichalski

We can't turn off some of default ftp:// http:// https:// mailto: file:// ?

No, we can't.
But why do you need to turn off the above schemes?

@ArkadiuszMichalski
Copy link
Contributor

I misread the above post and thought that you want add possibility to change all. Personally, I don't need it at the moment, but if someone asks about it, I will direct them to this topic.

@ArkadiuszMichalski ArkadiuszMichalski added the URL parser Issue related to the new URL parser label Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
URL parser Issue related to the new URL parser
Projects
None yet
Development

No branches or pull requests

7 participants