-
Notifications
You must be signed in to change notification settings - Fork 296
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
Feature: enable/disable permalinks for RSS #63
Conversation
I have noticed that in function showATOM, the
Should that piece of code also be made aware of the configuration flag? (I haven't tested to run this piece of code myself) Also, looking at the place where the
I guess one of the 2 should set it to true? |
You're 100% right on the showATOM, the bug report mentioned RSS and I didn't check if it was used elsewhere. Will fix that. I'm not sure that rainTPL will allow me to dynamically set the RSS/ATOM feed with variables. Will check that too. Thanks for the feedback. |
@@ -890,7 +890,8 @@ function showRSS() | |||
|
|||
// $usepermalink : If true, use permalink instead of final link. | |||
// User just has to add 'permalink' in URL parameters. e.g. http://mysite.com/shaarli/?do=rss&permalinks | |||
$usepermalinks = isset($_GET['permalinks']); | |||
// Also enabled through a config option | |||
$usepermalinks = isset($_GET['permalinks']) || !$GLOBALS['enablersspermalink']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder to update the configuration section of the docs (https://github.com/shaarli/Shaarli/wiki#configuration) when this gets merged. The default value should also be explicitely set (default to true
?) in the config section of index.php
I'm ok to merge this. I'd like some other reviews before going forward. |
@pikzen can you rebase this on top of current |
@pikzen please rebase, so that we can merge your changes. |
The branch has been rebased, along with a slight patch |
Ok to merge? |
@@ -10,7 +10,7 @@ | |||
<a href="?do=changetag"><b>Rename/delete tags</b> <span>: Rename or delete a tag in all links</span></a><br><br> | |||
<a href="?do=import"><b>Import</b> <span>: Import Netscape html bookmarks (as exported from Firefox, Chrome, Opera, delicious...)</span></a> <br><br> | |||
<a href="?do=export"><b>Export</b> <span>: Export Netscape html bookmarks (which can be imported in Firefox, Chrome, Opera, delicious...)</span></a><br><br> | |||
<a class="smallbutton" onclick="alert('Drag this link to your bookmarks toolbar, or right-click it and choose Bookmark This Link...');return false;" href="javascript:javascript:(function(){var%20url%20=%20location.href;var%20title%20=%20document.title%20||%20url;window.open('{$pageabsaddr}?post='%20+%20encodeURIComponent(url)+'&title='%20+%20encodeURIComponent(title)+'&description='%20+%20encodeURIComponent(document.getSelection())+'&source=bookmarklet','_blank','menubar=no,height=390,width=600,toolbar=no,scrollbars=no,status=no,dialog=1');})();"><b>✚Shaare link</b></a> <a href="#" style="clear:none;"><span>⇐ Drag this link to your bookmarks toolbar (or right-click it and choose Bookmark This Link....).<br> Then click "✚Shaare link" button in any page you want to share.</span></a><br><br> | |||
<a class="smallbutton" onclick="alert('Drag this link to your bookmarks toolbar, or right-click it and choose Bookmark This Link...');return false;" href="javascript:javascript:(function(){var%20url%20=%20location.href;var%20title%20=%20document.title%20||%20url;window.open('{$pageabsaddr}?post='%20+%20encodeURIComponent(url)+'&title='%20+%20encodeURIComponent(title)+'&description='%20+%20encodeURIComponent(document.getSelection())+'&source=bookmarklet','_blank','menubar=no,height=390,width=600,toolbar=no,scrollbars=no,status=no,dialog=1');})();"><b>✚Shaare link</b></a> <a href="#" style="clear:none;"><span>⇐ Drag this link to your bookmarks toolbar (or right-click it and choose Bookmark This Link....).<br> Then click "✚Shaare link" button in any page you want to share.</span></a><br><br> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Messed up indentation
I haven been able to test, as I'm AFK due to vacation. But looking at the code changes, it makes sense to me. OK to merge (after fixing this minor indentation issue) if someone else has been able to run this branch succesfully. |
913a0d2
to
8105a24
Compare
The option to see the shortlinks or permalinks has been added to the configuration panel. It is a simple checkbox This option is disabled by default (meaning that shortlinks are the default) Updated writeConfig() to save this option Also fixed a slight typo in config.html. Removed useless CSS & fixed a comment Enabled permalinks for the ATOM feed and fixed the isPermaLink attribute for the <guid> tag Reverted to default behavior and clarified its meaning EnableRssPermalinks is an oddly behaving option: when enabled, it shows a permalink in the description and a full link in the element title, and swaps it around when disabled. This clarifies the option for end-users Also, moved enable_rss_permalinks to $GLOBALS['config'] because it is a config option. fix indent
February 7th, I finally managed to fix an indentation issue. 75ish days for basically 4 characters. I've seen more efficient. Rebased on master, branch is mergeable |
New option ENABLE_RSS_PERMALINKS: choose whether the RSS item title link points directly to the link, or to the entry on Shaarli (permalink)
Tested and approved, merging. I'll add this option to the wiki. Maybe the description text on the settings page needs a small clarification, we'll see. Thanks! |
The option to see the shortlinks or permalinks has been added to the configuration panel. It is a simple checkbox
This option is disabled by default (meaning that shortlinks are the default)
Updated writeConfig() to save this option
Also fixed a slight typo in config.html.